-
Notifications
You must be signed in to change notification settings - Fork 40
Change object/morphism types in model notebooks #895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
ed42237 to
afb459e
Compare
36283e9 to
6b18588
Compare
6b18588 to
c8b4109
Compare
c8b4109 to
6d6a83a
Compare
epatters
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Matt for taking this on!
I have one high-level comment about the architecture, reflected in several of the comments below. Rather than using the cell constructor mechanism to construct a whole new cell and then extract its object/morphism type, we should, in each object declaration cell, create a cell switcher by looking up the available object types directly from the theory, and similarly for the morphism declaration cells. No code in the notebook directory should make reference to theories or models.
| } | ||
|
|
||
| function modelCellConstructors(theory: Theory): CellConstructor<ModelJudgment>[] { | ||
| function modelCellConstructors( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it makes sense to implement this feature by changing modelCellConstructors. As the name suggests, "cell constructors" are functions that construct new notebook cells. This is quite different in scoping to mutating existing cells of a given type.
| "tag" in content && | ||
| content.tag === "morphism" | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A hint that this design is off is that notebook editor is not supposed to know anything about theories, models, etc. It's a completely generic component.
Closes #184.