Skip to content

fix(editor): show the preview without making the user save first - #421

Merged
PathGao merged 1 commit into
masterfrom
fix/preview-without-saving
Aug 3, 2026
Merged

fix(editor): show the preview without making the user save first#421
PathGao merged 1 commit into
masterfrom
fix/preview-without-saving

Conversation

@PathGao

@PathGao PathGao commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Addresses the second item of #168 (thanks @dayeggpi):

allow user to switch to rendered view without saving/creating file … no way to see rendered view until file is saved

The prompt was never about losing data

Leaving the editor called loadMarkdown(tab.path, …), which re-reads the file from disk. On a dirty tab that would show the wrong text, so the code first resolved a save decision — write silently, or put up a modal.

Nothing is lost by switching view mode: the buffer stays in memory either way. The save existed to keep the screen from lying, not to protect the document. And the untitled branch two lines below already rendered the buffer instead:

tab.isEditing = false;
if (tab.path !== '') {
    await loadMarkdown(tab.path, { preserveEditState: true });   // reads disk
} else {
    renderMarkdownPreview(tab.rawContent, '');                   // renders buffer
}

#407 already extracted renderTabPreviewFromRaw(tab) for the print path — the same operation with a real path. Both exits now use it. The parts are all there; this is one wire.

git log -S places the modal in 85c6e5e fix: flush dirty tabs on window close — a window-close fix. The correct pattern for "don't lose edits when the buffer disappears" was applied in the same commit to a case where the buffer does not disappear.

Three more problems the disk read carried

  • The exit ran with isEditing already false, so it took the 50 KB preview branch: leaving the editor on a large file re-truncated a complete buffer, flagged it isTruncated (saves refused during that window), then re-read in the background.
  • loadMarkdown always writes into the active tab, so toggleSplitView(tabId) on a background tab would have pulled the active tab's content over. No caller reaches that today; the trap is gone.
  • if (!success) return kept the tab in edit mode when the write failed. saveContent returns false permanently for a read-only path or a lossily decoded buffer, so reading mode was unreachable for that tab, forever. A failed write is not a reason to hide the user's own text.

What was kept, and why

Segment Verdict
the silent flush Kept, narrowed to autoSave && !confirmBeforeSave. It is load-bearing for an independent reason: the auto-save effect treats a tab as writable only while isEditing || isSplit and clears its pending timer otherwise, so leaving edit mode drops the scheduled write. This is the last flush before that window closes — not a condition of the switch
silentSave forcing a write when auto-save is off Removed. Its only job was suppressing the modal on the hotkey path; with no modal its residue was "Cmd+E writes the file even though you turned auto-save off" — a hidden write contradicting the user's own setting, and an asymmetry with the toolbar button
the askCustom modal Deleted. 100 % "because we read the disk". Its discard option reverted the buffer to originalContent — a destructive choice offered for a switch that destroys nothing. confirmBeforeSave promises confirmation before each write; with no write there is nothing to confirm
the TOCTOU savedNewerEdits toast Kept, moved inside the flush branch. Its return is gone: the three reasons for it are stale (Cmd+S in reading mode was fixed earlier in this same #168 round; a dirty tab is already protected from disk reload by resolveExternalChange; auto-save being off is now the intended steady state). What survives is the real signal — after a flush, a still-dirty tab means the file is one revision behind

What loadMarkdown also did

Traced line by line; it is not modified. Lost and judged safe: setShowHome(false) (unreachable state), the load-revision bump (canApplyFullLoad already requires a clean tab), setTabDecodedLossy (entering edit mode sets it, and not re-reading means the verdict has not changed). Gated and never fired: resetScrollHistory, navigate, updateLoading. Must not have run: setTabRawContent, which replaces originalContent and clears isDirty — it would have erased the edits, and the dirty short-circuit would have left a stale preview anyway.

One deliberate behaviour delta: a view toggle no longer bumps the file in Recent. The entry exists from the actual open; a mode toggle is not an open.

Closing still asks

canCloseTab and appExit are byte-for-byte untouched, and two tests assert they still prompt — executed against the real canCloseTab. Those buffers are about to cease to exist. This one is not.

Mainstream agrees, and on the shape rather than the detail — VS Code follows the in-memory document and previews an untitled buffer; Obsidian makes reading view a plain toggle on the same key Markpad binds, documenting no save step; Typora has no preview/file distinction at all. No new "this is unsaved" indicator: the tab's dirty dot is the standing signal, and adding a reading-mode banner would be a Markpad-only invention for a state every other editor treats as unremarkable.

Tests

scripts/viewModeWithoutSaving.test.ts extracts the real toggleEdit / toggleSplitView / renderTabPreviewFromRaw from the component with a string- and comment-aware brace slicer, transpiles with the project's own typescript, and runs them against the real TabManager. The fake disk reproduces loadMarkdown's dirty short-circuit and serves text that differs from the buffer, so a disk route shows up in the rendered output, not just in a call log.

baseline 5 pass / 7 fail
final 12 / 12

Sample reds: no unsaved-changes modal on a view toggle → actual ['modal.youHaveUnsavedChangesBeforeReturning'] · the file is not re-read to leave the editor → actual ['/notes/note.md'] · but the view still switchestrue !== false (the unwritable-file trap).

The 5 green on both sides are the boundary: closing a tab still asks and Cancel still keeps it open, closing the window still reviews unsaved tabs, auto-save still flushes, untitled still never hits the Save dialog.

npm run check   436 files, 0 errors
npm test        521 / 521
cargo test      131 / 131

Orphaned keys, not deleted

modal.youHaveUnsavedChangesBeforeReturning and modal.youHaveUnsavedChangesBeforeClosingSplitView now have zero usages but are still defined in 26 locales. i18nCoverage stays green (it fails on keys English lacks, not on unused ones). Left for whoever prunes the dictionary.

Not covered

  • A double toast on a refused lossy save: the flush failure path emits toast.autoSaveFailed on top of the guard's own explanation. The background auto-save suppresses this via isLossySaveRefused; the toggle path never did, before or after. Pre-existing, but slightly more visible now that the transition proceeds instead of stopping.
  • External changes during the self-write grace window are now entirely the watcher's business; the mode toggle no longer incidentally picks them up. Untested interaction.
  • Verification is unit-level against the real store; no end-to-end run.

🤖 Generated with Claude Code

Leaving the editor re-read the file from disk, so a dirty tab had to
resolve a save decision - silently write, or answer a modal - before the
rendered view could appear. dayeggpi reported this in #168: "no way to
see rendered view until file is saved."

The prompt was never about losing data. Nothing is lost by switching
view mode; the buffer stays in memory either way. It existed because the
exit path called `loadMarkdown(tab.path)`, and reading the disk on a
dirty tab would show the wrong text. The untitled branch two lines below
already rendered the buffer instead - and #407 extracted
`renderTabPreviewFromRaw` for the print path, which is exactly the same
operation with a real path. Both exits now use it.

That also removes three problems the disk read carried: the exit took
the 50KB preview branch, so leaving the editor on a large file
re-truncated a complete buffer and refused saves until the background
read finished; `loadMarkdown` writes into the *active* tab, so
`toggleSplitView(tabId)` on a background tab would have yanked the
active one; and `if (!success) return` kept the tab in edit mode when
the write failed - which for a read-only file or a lossily decoded
buffer meant reading mode was permanently unreachable.

One segment of that block is kept, narrowed to `autoSave &&
!confirmBeforeSave`: the auto-save effect treats a tab as writable only
while `isEditing || isSplit`, and clears its pending timer otherwise, so
leaving edit mode drops the scheduled write. That flush is the last
chance before the window closes, not a condition of the switch.

Closing a tab and closing a window still ask. Those buffers are about to
cease to exist; this one is not. VS Code, Obsidian and Typora all render
the buffer, and none of them asks.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@PathGao
PathGao merged commit 818492b into master Aug 3, 2026
4 checks passed
@PathGao
PathGao deleted the fix/preview-without-saving branch August 3, 2026 06:14
PathGao added a commit that referenced this pull request Aug 3, 2026
…#441)

`Tab` carries three same-shaped strings. `rawContent` is the Markdown, and
it is what the editor edits, what reaches disk, and what the exports read.
`content` is the rendered preview HTML, injected via `{@html}`. Two places
read the wrong one.

1. TitleBar.svelte gated Export as HTML / Export as PDF on
   `tabManager.activeTab.content`, while `exportAsHtml` gates on
   `ctx.rawContent`. `content` is only refreshed while the preview is on
   screen — `tab.isSplit || (isEditing && settings.showToc)` — so for an
   untitled buffer being edited with the TOC closed it is still `''` while
   `rawContent` holds the user's text. `showToc` and `newFileDefaultMode`
   default such that Ctrl+T then typing is exactly that state, and the menu
   hid two commands that would have produced a file. Reachable since #421
   let the preview work without saving first.

2. `addTab(path, content = '')` assigned its argument to `content`,
   `rawContent` and `originalContent` alike — coherent when the three were
   one field, but it let Markdown into the field that is injected as HTML.
   Both callers in the app pass `''`, so nothing shipped broken; the value
   is sanitized at the sink either way. The parameter is now `rawContent`
   and `content` starts empty, as at every other Tab construction site.

scripts/renderedHtmlField.test.ts evaluates the real gate, the real
refresh condition and `exportAsHtml`'s real precondition against the real
TabManager. Four of its nine tests are red on master.

Co-authored-by: PathGao <gaoyanbo@gaoyanbodeMacBook-Air.local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
PathGao added a commit that referenced this pull request Aug 8, 2026
* feat(preview): Edit jumps to the fragment you right-clicked (#90)

"Edit" in the preview's context menu opened the editor wherever the tab
was last left. It now opens it on the block, inline element or selection
that was right-clicked, with those lines selected.

No new preview-to-source mapping. comrak already renders with
`options.render.sourcepos = true`, and `previewAnchor.ts` is already the
module that reads those ranges for the tab's reading position and for
split-view scroll sync; this adds two small functions there — the climb
to the narrowest annotated ancestor, and the union of a selection's two
ends — and reuses `parseSourceposLineRange` for the parsing.

Nor a new jump. `revealHeader` already revealed and selected a source
line for the outline; its line branch now delegates to
`revealSourceRange`, which both callers share, so the two cannot scroll
or focus differently. That function also gained the clamp `revealHeader`
never had: `getLineMaxColumn` throws past the end of the model, and both
callers can hand over a line from a render of a buffer that has since
got shorter.

Lines only, never columns. `data-sourcepos` describes the text
`convert_markdown` hands comrak, and only its line numbers are
contractually equal to the raw buffer's — the math mask substitutes a
token of a different length, so columns after it on the line drift.

The selection is the highlight the report asks for: Monaco draws it in
the theme's own colour, and it clears itself on the next click or
keystroke, so no decoration, CSS or timer is needed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(preview): shift renderer line numbers onto the buffer, and carry the selection into ⌘E

Two follow-ups from testing the jump by hand.

**The jump landed short.** `renderMarkdownPreview` hands comrak
`getMarkdownBodyWithoutFrontMatter(raw)`, so every `data-sourcepos`
counts from the first line of the BODY. The editor holds the whole
file. In a document with front matter every jump was early by its
height — 11 lines in `samples/stress-test.md`.

Nothing in the attribute says which of the two numberings it means, and
the difference is invisible in a document without front matter, which
was every fixture in the suite.

**The outline has had the same bug for as long as both have existed.**
`Toc.svelte` reads the same attribute and passes it to `revealHeader`
untouched, so clicking a heading in a document with front matter has
always landed short too. It went unnoticed because `revealHeader` falls
back to a text search when the line is null, and that path is correct.

Both now go through `toBufferRange`. Worth noting the task-checkbox
write-back is *not* affected: it counts lines in the body as well, so
both sides of that comparison share one numbering.

**⌘E and the toolbar now carry the selection too.** The context menu's
"Edit" already opened the editor on what you had selected; the entry
points people actually use did not. `toggleEdit` resolves the selection
before it flips `isEditing` — reading after would ask whether the reader
was in the editor rather than in the preview — and arms the jump only if
the switch took, since the read can fail and leave the tab in reading
mode. `editSourceRange` passes `revealSelection: false` because it has
already captured its target: clicking a menu item is how a selection
goes away.

Tests cover the arithmetic (including CRLF, a `---` that is not front
matter, and the real stress document) and, separately, that all three
call sites route through the shift — pinning the arithmetic alone would
pass with a call site still handing over a raw body line.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(view): one meaning for ⌘E, and end the jump highlight on any move

**⌘E and the preview's "Edit" are now the same move.** They were two
implementations of one intent, and they disagreed: "Edit" carried the
reader to the fragment they had selected, the chord did not, and in
split view the chord did nothing at all. `toggleEditView` is now what
⌘E means, and the hotkey, the toolbar, the title bar and Monaco's own
command all route through it — so the chord cannot mean one thing with
the caret in the editor and another with it in the preview.
`formatShortcutKeymap.test.ts` holds those two layers together and
caught the divergence while this was being written.

    reading            → editor, on the selected fragment
    editor (alone)     → back to reading
    split + selection  → jump to it; the layout does not move
    split, no selection→ nothing

**The inert case is the considered one.** Split view already grants
what ⌘E asks for — the editor is on screen — and with no selection
there is no fragment to travel to, so every remaining reading of the
chord is a layout change nobody requested. A mistyped ⌘E would cost the
reader the preview pane and a keystroke to get it back; doing nothing
costs nothing. ⌘\ opens and closes the split and stays the only way.

`toggleEdit` goes back to being only a mode switch. Resolving the
selection lives in `toggleEditView`, above every path that flips
`isEditing`, because a selection read after the switch answers for the
editor rather than for the preview the reader was looking at.

**The outline's jump highlight could not be dismissed.** It is a
temporary emphasis, but the only thing that removed it was a scroll
event on the preview — and the jump's own smooth scroll is swallowed by
`clickLock`, so unless the reader scrolled again afterwards it stayed
until the document was re-rendered. A pointerdown in the preview and a
keydown anywhere now end it too, both bypassing `clickLock`: that lock
exists to ignore scrolling the app itself caused, and a deliberate
action by the reader is never that.

Both were found while testing #90 by hand. Neither is from that change
— the ⌘E branch is #421's and the highlight is older — but both are in
the paths it made people exercise.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: PathGao <gaoyanbo@gaoyanbodeMacBook-Air.local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant