fix(editor): allow saving from reading mode - #365
Merged
Conversation
With a document that was never saved, Ctrl+S in reading mode did nothing at all. `toggleEdit` only runs its save-or-confirm flow for tabs that already have a path, so an untitled buffer switches to reading mode with its content still unsaved -- and the keydown handler then suppressed the one shortcut that could rescue it. The only way to keep that text was to switch back to the editor first, which is not something the shortcut's silence suggests. Saving is not mode-specific. The guard now asks whether there is anything to write rather than which pane happens to be visible. A saved, unmodified document stays a no-op, so the shortcut cannot churn its mtime and wake the file watcher, and the HOME tab is excluded by the same condition without being named. `documentSession.saveContent` already opens the Save dialog for a pathless tab, so reading mode needs no extra plumbing. `preventDefault` now runs unconditionally, including for the no-op case; previously reading mode let the shortcut fall through to the webview's own Save Page dialog. Reported by @dayeggpi in sftwrdotdev#168. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 3, 2026
Closed
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
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.
Summary
With a document that was never saved,
Ctrl/Cmd+Sin reading mode did nothing at all.toggleEditonly runs its save-or-confirm flow for tabs that already have a path (if (tab.isDirty && tab.path !== '')), so an untitled buffer switches to reading mode with its content still unsaved. The keydown handler then suppressed the shortcut behindif (isEditing || isSplit)— the one action that could rescue that text was the one action the mode refused. Nothing on screen suggests the shortcut is unavailable; it just silently does nothing, and the only recovery is knowing to switch back to the editor first.Saving is not mode-specific. The guard now asks whether there is anything to write rather than which pane happens to be visible:
documentSession.saveContentalready opens the Save dialog for a pathless tab, so reading mode needs no extra plumbing;preventDefaultnow runs unconditionally, including for the no-op case. Previously reading mode let the shortcut fall through to the webview's own Save Page dialog.Credit
Reported by @dayeggpi in #168. That PR also proposed two other changes, both since overtaken by other work: the "autosave + confirm before save prompts every 1.5 s" loop is fixed on
master(auto-save is now disabled outright whileconfirmBeforeSaveis on), and the menu Save entry it edited no longer exists — the macOS File submenu was removed in a later refactor. This lifts the one finding that is still live.Validation
npm run check— 0 errors, 0 warningsnpm test— 168/168, including a newscripts/saveFromReadingMode.test.tscovering the untitled case, the clean-document no-op, unconditionalpreventDefault, and HOME staying unsavable