Stop the menu offering what an edit will not let it do - #28
Merged
Merged
Conversation
#22: while a value edit was live the menu stayed fully active, and thirteen of its rows did nothing when picked — the row highlighted, the menu closed, and nothing happened or was said. Each command guards itself with an early return on ui.Editing, which is the right defence and an invisible one; the menu is built once and was never told the world had changed. Fifteen rows now go disabled for the life of an edit, flipped at the same two chokepoints containment uses, and registered next to the command each one calls so the set cannot drift from the guards it mirrors. The other half of that table is registered too — Edit Value, Cut, Copy, Paste, Save, Quit, Toggle Light/Dark stay enabled, because disabling those would be the same bug pointing the other way, and a drill check now says so by name. The keyboard had the identical hole and no affordance at all: ^B mid-edit was exactly as silent as the row it mirrors. The guards now answer through the value pane's border title, which is already the channel for transient status, so a chord and its menu row give the same answer. Save is the exception, and does not go disabled: it commits the edit and then writes the file. Save while typing means "commit this and write it" to anyone mid-paragraph, and refusing silently was this issue's worst case — no save, no error, no dirty marker cleared, and every reason to believe the work was on disk. The commit goes through the UndoManager like any other edit, so it is undoable. Quitting had the same shape of hole from the other side. ConfirmDiscard gates on _model.Dirty, which tracks the DOM, and an uncommitted edit has not reached the DOM — so on an otherwise-clean document ^Q asked nothing and the paragraph went with the session. The gate now counts a live edit as unsaved work: Save commits and writes, Discard drops it because the user said so, and Cancel leaves the edit exactly as it was. It deliberately does not commit before asking, which would make Cancel destructive. Drilled on all four CI fixtures (356/343/335/335, all green) and mutation-tested: not flipping the inert rows, disabling the live ones too, restoring Save's silent return, and restoring the old quit gate each turn red on the checks that name them, and only those. One correction to the issue's reasoning: it expected Theme.Content.Disabled to render the dim row. The menu popover does not take fux's theme at all — it draws in Terminal.Gui's own scheme — but disabled rows still render visibly dimmer there, measured through a PTY as fg (54,69,79) enabled against (74,95,109) disabled on the same background. The affordance lands; the mechanism is not the one predicted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016X4pgrsMBcsXjqWDo2gBBt
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.
Closes #22. Completes Batch A, after #27 closed #24 and #20.
While a value edit is live the menu stayed fully active and thirteen of its rows did nothing
when picked: the row highlighted, the menu closed, and nothing happened or was said. Each
command guards itself with an early return on
ui.Editing— the right defence, and aninvisible one. The menu is built once and was never told the world had changed.
What changed
Fifteen rows go disabled for the life of an edit, flipped at the same two chokepoints
containment uses (
StartValueEdit/EndValueEdit), and registered next to the command eachone calls so the set cannot drift from the guards it mirrors.
The rows that stay live are registered too — Edit Value, Cut, Copy, Paste, Save, Quit,
Toggle Light/Dark. Disabling those would be the same bug pointing the other way, and a drill
check now says so by name.
The keyboard had the identical hole, with no affordance at all:
^Bmid-edit was exactlyas silent as the row it mirrors. The guards now answer through the value pane's border title,
already the channel for transient status, so a chord and its menu row give the same answer.
Save is the exception and commits, per your call. It is not disabled: it commits the edit
and then writes the file. Refusing silently was the worst case here — no save, no error, no
dirty marker cleared, and every reason to believe the paragraph was on disk. The commit goes
through the UndoManager like any other edit, so it is undoable.
Quitting had the same hole from the other side, and this PR closes it too.
ConfirmDiscardgates on
_model.Dirty, which tracks the DOM, and an uncommitted edit has not reached the DOM —so on an otherwise-clean document
^Qasked nothing and the typed paragraph went with thesession. The gate now counts a live edit as unsaved work: Save commits and writes,
Discard drops it because the user said so, and Cancel leaves the edit exactly as it was.
It deliberately does not commit before asking, which would make Cancel destructive. The issue
listed this as a separate fix — flagging it as slightly beyond the letter of #22, but it is one
line of gate and the same family of silent loss as #24.
Verification
--drillon all four CI fixtures: 356 / 343 / 335 / 335, all PASS. Every new check ismutation-tested and goes red only where it should:
an edit disables every row that cannot run during one,…Edit ▸ Snippet… among them…and leaves enabled the rows that still mean something,…Edit ▸ Cut among those^S during an edit commits it,…writes it to the DOM,…the typed value is in the file on disk^Q on an uncommitted edit prompts rather than discarding it (depth was -1)One correction to the issue
#22 expected
Theme.Content.Disabled(base01 on base03) to render the dim row. The menu popoverdoes not take fux's theme at all — it draws in Terminal.Gui's own scheme. Disabled rows still
render visibly dimmer there: measured through a PTY, fg
(54,69,79)enabled vs(74,95,109)disabled on the same background. The affordance lands; the mechanism is not the one predicted.
Theming the popover is a separate question and is not touched here.
🤖 Generated with Claude Code
https://claude.ai/code/session_016X4pgrsMBcsXjqWDo2gBBt