Skip to content

Commit a1fa646

Browse files
committed
fix(save): harden format-on-save against tab-switch races and failure masking
- Propagate formatter.format()'s boolean through acode.format() (true = ran, false = failed, null = no formatter) so a handled formatter failure aborts the save instead of writing unformatted content as a successful save. - Revalidate the active file after the formatter resolves in acode.format(); a mid-format tab switch now reports failure. - Abort the format dispatch itself when the tab switched mid-format: prettierFormatter and LSP formatDocument return false without writing instead of dispatching into the newly active document. - Run formatting before Save As/new-file target mutations (file creation, URI reassignment, recents updates) so a failed format aborts before any of them happen.
1 parent 64efbf4 commit a1fa646

4 files changed

Lines changed: 45 additions & 22 deletions

File tree

‎src/cm/lsp/clientManager.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,13 @@ export class LspClientManager {
538538
plugin.client.sync();
539539
return true;
540540
}
541+
// The user may have switched tabs while the formatting request was
542+
// in flight; never apply this file's edits to another file's
543+
// document. Abort without writing instead.
544+
const em = (globalThis as Record<string, unknown>).editorManager as
545+
| { activeFile?: unknown }
546+
| undefined;
547+
if (em && file && em.activeFile !== file) return false;
541548
const applied = applyTextEdits(plugin, view, edits);
542549
if (applied) {
543550
plugin.client.sync();

‎src/lib/acode.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -871,8 +871,16 @@ class Acode {
871871
}
872872

873873
try {
874-
await formatter.format();
875-
return true;
874+
// Propagate the formatter's own result instead of assuming
875+
// success: formatters resolve `false` for handled failures
876+
// (e.g. Prettier failing to parse, LSP reporting failure).
877+
const didFormat = (await formatter.format()) !== false;
878+
// The formatter dispatches through the shared editor view; if
879+
// the user switched tabs while it was awaiting, its edits may
880+
// have targeted the wrong document, so report failure instead
881+
// of success and let callers abort.
882+
if (editorManager.activeFile !== file) return false;
883+
return didFormat;
876884
} catch (error) {
877885
helpers.error(error);
878886
return false;

‎src/lib/prettierFormatter.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,11 @@ export async function formatActiveFileWithPrettier() {
9898

9999
if (formatted === source) return true;
100100

101+
// The user may have switched tabs while the config/format awaits
102+
// resolved; never dispatch into a document that is no longer this
103+
// file's. Abort without writing instead.
104+
if (editorManager?.activeFile !== file) return false;
105+
101106
editor.dispatch({
102107
changes: {
103108
from: 0,

‎src/lib/saveFile.js‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,29 @@ async function saveFile(
117117
let saved = false;
118118

119119
try {
120+
if (appSettings.value.formatOnSave) {
121+
// Format the in-memory content BEFORE creating/reassigning the
122+
// save target below: a failed format aborts the save before any
123+
// file creation, URI reassignment, or recents update happens.
124+
// Formatters operate on the active editor document. If the user
125+
// switched tabs while an async save step was pending, formatting
126+
// here would rewrite the wrong file, so only format when the file
127+
// being saved is the active one.
128+
file.markChanged = false;
129+
try {
130+
if (editorManager.activeFile === file) {
131+
// Await the formatter so the write below captures the
132+
// formatted document. A failed format aborts the save;
133+
// `null` means no formatter is configured, which is not
134+
// a failure.
135+
const formatted = await acode.exec("format", false);
136+
if (formatted === false) return false;
137+
}
138+
} finally {
139+
file.markChanged = true;
140+
}
141+
}
142+
120143
if (isSaveAs || newUrl) {
121144
// if save as or new file
122145
const fileUri = Url.join(newUrl, file.filename);
@@ -171,26 +194,6 @@ async function saveFile(
171194
}
172195
}
173196

174-
if (appSettings.value.formatOnSave) {
175-
// Formatters operate on the active editor document. If the user
176-
// switched tabs while an async save step was pending, formatting
177-
// here would rewrite the wrong file, so only format when the file
178-
// being saved is the active one.
179-
file.markChanged = false;
180-
try {
181-
if (editorManager.activeFile === file) {
182-
// Await the formatter so the write below captures the
183-
// formatted document. A failed format aborts the save;
184-
// `null` means no formatter is configured, which is not
185-
// a failure.
186-
const formatted = await acode.exec("format", false);
187-
if (formatted === false) return false;
188-
}
189-
} finally {
190-
file.markChanged = true;
191-
}
192-
}
193-
194197
const savedDoc = file.session?.doc || null;
195198
const savedVersion = file.docVersion;
196199
const data = getDocText(savedDoc);

0 commit comments

Comments
 (0)