Skip to content

fix(conflict): closing cannot answer the question, and Compare shows the answer (#692) - #699

Merged
PathGao merged 1 commit into
masterfrom
fix/conflict-must-be-answered
Aug 21, 2026
Merged

fix(conflict): closing cannot answer the question, and Compare shows the answer (#692)#699
PathGao merged 1 commit into
masterfrom
fix/conflict-must-be-answered

Conversation

@PathGao

@PathGao PathGao commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

#697 and #698 are merged; this now sits directly on master.

What this is

The external-change bar asks a question. Two things stopped it from being answerable.

Closing the tab answered it, silently, in the destructive direction. canCloseTab handed a dirty tab straight to auto-save, which is on by default:

VS Code writes  →  the bar goes up: "Reload from disk" / "Keep my version"
                          │
      the user presses Ctrl+W without answering
                          │
                          ▼
        canCloseTab: dirty + autoSave  →  saveContent
                          │
                          ▼
        their version is overwritten, nothing said

A save is the answer "keep mine". Giving it on the user's behalf, to a question they were shown and did not answer, is the one direction that destroys somebody else's work. VS Code prompts here.

And the answer itself was invisible. "Reload from disk" replaces the buffer, and setValue clears the undo stack with it, so the bar asked for an irreversible decision with no way to see what would be lost. Two buttons, no diff. VS Code and Sublime both put a Compare beside the same two choices.

Mechanism

Closing. The disk is checked before the auto-save branch, and a file that moved gets the question instead:

const diskMoved = tab.path !== '' && (await fileDiffersFromBaseline(tab, tab.path));
if (settings.autoSave && tab.path !== '' && !diskMoved) { …unchanged… }
const response = await options.askClose(tab.title, diskMoved);

askClose takes the flag rather than a second dialog, because it is the same three answers — Save / Discard / Cancel — with a different meaning for Save. The wording swaps to the sentence the bar already uses ("This file changed on disk while you had unsaved changes"), so no string is minted for it and "you have unsaved changes" stops being said where it would hide the half that matters.

Answering Save calls allowOverwriteOnce first. Without that the write guard from #698 refuses, saveContent returns false, canCloseTab returns false, and the dialog has a Save button that does nothing and a tab that will not close.

Only reached with a dirty tab, so an untouched document still closes with no I/O and no question.

Compare. A read-only Monaco diff of the file against the buffer, opened from a third button on the bar, with both answers repeated once the difference is on screen. Monaco is dynamically imported exactly as Editor.svelte does it, so it costs nothing until the overlay opens and the chunk is already in the bundle by then.

The file is read when Compare is pressed, not when the bar went up: the bar can stand for a while, and what matters is the file as it is when the question is actually being answered.

The view creates its own models and disposes them. Handing it the tab's live model would put a second editor on a buffer Editor.svelte owns, and a read-only view of a document is not a place to change it from.

Scope

One read per close of a dirty tab, and one per Compare. Neither is on a hot path.

Three of the 26 locales carry the externalChange section at all; the rest already fall back to English for it, so compare, onDisk, mine and close are nine strings, not a hundred.

Not done: making Reload undoable. pushEditOperation instead of setValue would let Ctrl+Z resurrect the replaced buffer, and the existing comment in Editor.svelte explains why that is a separate change — it would also let undo resurrect a buffer the truncation and lossy-decode guards exist to keep away from the file. Compare addresses the same complaint from the other side: the decision is visible before it is irreversible rather than reversible after.

Not done: a three-way merge. Obsidian does one (diff-match-patch) and it is the better answer where it applies; it is also a different feature, and this bar is about a choice between two versions.

Not done: liveMode still defaults to off.

Tests

scripts/externalChangeReload.spec.ts, seven added.

Behavioural, against the fake disk #698 introduced:

  • closing a tab whose file changed asks, and is told the disk moved, and writes nothing behind the question
  • answering Save to that dialog does overwrite, rather than failing silently — the case the authorisation exists for
  • closing an untouched file still just saves and closes, with nothing asked

Source-shape, for the parts that need a Svelte runtime:

  • the bar offers Compare beside the two answers
  • Compare reads the file when it is pressed, and diffs it against the buffer
  • both answers close the comparison with them, so the overlay cannot outlive its question
  • the diff view creates its own models, is read-only on both sides, and disposes what it created

Checked by breaking what they guard: pinning diskMoved to false turns two red; dropping the two comparison = null lines turns one red.

Verification

npm audit             0 vulnerabilities
npm run check         815 files, 0 errors, 0 warnings
npm test              963 pass, 0 fail
npm run test:vitest   44 files, 384 pass
npm run build         clean

npm run test:vitest also reports 14 failures in this environment (session-restore and window-tag snapshots, assert.deepEqual reference-identity under node 26 + jsdom), byte-identical on a clean checkout of the base branch.

cargo test not re-run: no Rust changed here.

Not verified by hand: how the diff overlay looks. It is a createDiffEditor with the app's own Monaco theme in a fixed-position panel, and the build is clean, but nobody has opened it. That is the one part of this PR a test cannot speak for.

@PathGao
PathGao force-pushed the fix/ask-the-file-not-the-clock branch from ca532ef to dc311fd Compare August 21, 2026 18:09
@PathGao
PathGao force-pushed the fix/conflict-must-be-answered branch from 4bee9e1 to 571a571 Compare August 21, 2026 18:09
Base automatically changed from fix/ask-the-file-not-the-clock to master August 21, 2026 18:41
…the answer

Two ways the external-change bar could not actually be answered.

Closing a tab answered it silently, in the destructive direction. `canCloseTab`
handed a dirty tab straight to auto-save, which is on by default, and a save is
the same answer as "keep mine" — given on the user's behalf to a question they
were shown and did not answer, destroying the other program's write. The disk
is checked before that branch now, and a file that moved gets the question
instead, carrying the wording the bar already uses. Answering Save there also
carries the authorisation the write guard waits for, or Save would do nothing
and the tab would refuse to close.

And the answer itself was invisible. Reload replaces the buffer, and `setValue`
clears the undo stack with it, so the bar asks for an irreversible decision
without showing what would be lost. VS Code and Sublime both put a Compare
beside the same two choices; this adds it, as a read-only Monaco diff of the
file against the buffer, with both answers repeated once the difference is on
screen. The file is read when Compare is pressed rather than when the bar went
up, because the bar can stand for a while.

The diff view builds models of its own and disposes them: handing it the tab's
live model would put a second editor on a buffer Editor.svelte owns.

Three of the 26 locales carry these strings; the rest already fall back to
English for this section.
@PathGao
PathGao enabled auto-merge (squash) August 21, 2026 18:43
@PathGao
PathGao force-pushed the fix/conflict-must-be-answered branch from 571a571 to 8193eeb Compare August 21, 2026 18:43
@PathGao
PathGao merged commit 4f6fc7b into master Aug 21, 2026
4 checks passed
@PathGao
PathGao deleted the fix/conflict-must-be-answered branch August 22, 2026 05:05
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