fix: a read-only diagram can still be laid out - #49
Merged
Conversation
`edits: 'read-only'` registered no operation handlers at all. That reads "read-only" as "no operations", when what it has to mean is "no operations that change the SOURCE". Laying a diagram out and moving a node are operations, and neither touches the document the diagram was generated from. They write to the separate layout file, which is presentation state. Refusing them bought nothing and cost a great deal: the context menu offers its three layout entries unconditionally, so all three appeared on a read-only diagram and all three did nothing when clicked, because a GLSP operation with no bound handler fails silently. Dragging a node did not persist either, so an arrangement made by hand to compensate was gone by the next open. That fell hardest on exactly the diagrams least able to absorb it. A read-only diagram is usually a generated one, and a generated diagram is the kind that most needs laying out, because nobody placed its nodes by hand to begin with. The read-only cut now sits below the neutral in-core handlers and above the consumer's injected modules, which is where source editing actually lives. The guarantee that matters is unchanged and is now the load-bearing one: `ReadOnlySourceModelStorage.saveSourceModel` is a no-op, so nothing here widens what a reader can alter — only what they can rearrange. Two existing tests pinned the old rule and are updated rather than deleted, because both were asserting more than their subject. The module-injection suite now asserts that read-only injects none of the source-editing handlers, which is what that file is about; the edit-strategy suite asserts that both session kinds bind the same presentation handlers and differ only by what the consumer injects. Each names the layout handlers one by one, since the regression to catch is a single menu entry going quiet rather than all of them. This also leaves the seam in the right shape for a product that later wants to edit through the diagram: what it would add is an operation module, on the other side of a cut that now falls in a defensible place.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
edits: 'read-only'registered no operation handlers at all.That reads "read-only" as "no operations", when what it has to mean is "no operations that change the source". Laying a diagram out and moving a node are operations, and neither touches the document the diagram was generated from — they write to the separate layout file, which is presentation state.
Refusing them bought nothing and cost a lot. The context menu offers its three layout entries unconditionally, so on a read-only diagram all three appeared and all three did nothing when clicked: a GLSP operation with no bound handler fails silently. Dragging a node did not persist either, so an arrangement someone made by hand to compensate was gone by the next open.
This fell hardest on the diagrams least able to absorb it. A read-only diagram is usually a generated one, and a generated diagram is the kind that most needs laying out, because nobody placed its nodes by hand to begin with.
The change
The read-only cut moves down. It now sits below the neutral in-core handlers and above the consumer's injected modules, which is where source editing actually lives.
The guarantee that matters is unchanged, and is now the load-bearing one:
ReadOnlySourceModelStorage.saveSourceModelis a no-op. This widens what a reader can rearrange, never what they can alter.Existing tests
Two pinned the old rule. Both are updated rather than deleted, because both asserted more than their subject:
Each names the layout handlers one by one, since the regression worth catching is a single menu entry going quiet rather than all of them.
Verified
npm run build,npm run check:neutrality(5/5 gates),npm test— all green.packages/diagram-server/test/read-only-layout-operations.test.ts.Note
This leaves the seam in the right shape for a product that later wants to edit through the diagram: what it would add is an operation module, on the other side of a cut that now falls in a defensible place.