diff --git a/perf/budgets.json b/perf/budgets.json index 5f3110ab2..663902ce3 100644 --- a/perf/budgets.json +++ b/perf/budgets.json @@ -73,6 +73,7 @@ "tabs.openSmallMs": 100, "tabs.switchP95Ms": 33, "tabs.switcherP50Ms": 33, + "tabs.viewsRebuilt": 0, "agentTabs.conversationNodes": 20000, "agentTabs.rowsAlwaysRendered": 0, diff --git a/perf/scenarios.ts b/perf/scenarios.ts index 4e6473cbe..6ed8cba12 100644 --- a/perf/scenarios.ts +++ b/perf/scenarios.ts @@ -449,12 +449,24 @@ const tabs: Scenario = { await h.openFile(`${ROOT}/src/tabs/${name}`, tsSource(2_000, `tab-${i}`)); } const switches: number[] = []; + // Recently shown tabs keep their editor view: showing one again must not build a new one. + let viewsRebuilt = 0; + let switchWork = await h.counters(); for (let round = 0; round < 3; round++) { + if (round === 1) switchWork = await h.counters(); for (let i = 0; i < names.length; i++) { const tab = h.page.getByRole("tab", { name: names[i], exact: true }); switches.push(await h.measure(() => tab.click(), ".cm-content", `// tab-${i}`)); + const fresh = await h.page.evaluate(() => { + const view = document.querySelector("[data-editor-host] .cm-editor") as { perfSeen?: boolean } | null; + if (!view || view.perfSeen) return false; + view.perfSeen = true; + return true; + }); + if (round > 0 && fresh) viewsRebuilt++; } } + const work = Harness.delta(switchWork, await h.counters()); // The Switcher (Ctrl+Tab) back and forth between the two most recent tabs. const before = await h.counters(); const switcher: number[] = []; @@ -463,12 +475,14 @@ const tabs: Scenario = { switcher.push(await h.measure(() => h.pressAction("navigate.switcher"), ".cm-content", `// tab-${tag}`)); await h.page.waitForTimeout(150); } - const work = Harness.delta(before, await h.counters()); + const switcherWork = Harness.delta(before, await h.counters()); return { openSmallMs, ...latencySummary(switches, "switch"), + switchMainThreadMs: work.taskMs / (names.length * 2), + viewsRebuilt, ...latencySummary(switcher, "switcher"), - switcherMainThreadMs: work.taskMs / switcher.length, + switcherMainThreadMs: switcherWork.taskMs / switcher.length, }; }, }; diff --git a/src/editor/api.ts b/src/editor/api.ts index 6be4d04eb..25f99448d 100644 --- a/src/editor/api.ts +++ b/src/editor/api.ts @@ -28,7 +28,7 @@ import { type RangeSet, type Transaction, } from "@codemirror/state"; -import { GutterMarker, ViewPlugin, type EditorView } from "@codemirror/view"; +import { GutterMarker, ViewPlugin, type EditorView, type ViewUpdate } from "@codemirror/view"; import type { LanguageDescription } from "@codemirror/language"; import type { LineEnding } from "@/platform/ipc"; import { create } from "zustand"; @@ -402,6 +402,17 @@ export interface ContributedIntention { /** A background provider has new context actions for the current document. */ export const intentionsChanged = StateEffect.define(); +/** + * A hidden editor view is shown again. Recently used tabs keep their view, detached from the + * document, while hidden; plugins that skip work while `view.dom` is disconnected redo it when + * `shownAgain`. + */ +export const viewShown = StateEffect.define(); + +export function shownAgain(u: ViewUpdate): boolean { + return u.transactions.some((tr) => tr.effects.some((e) => e.is(viewShown))); +} + /** Local fixes of a diagnostic range (hover link and Apply Quick Fix). */ export const quickFixProviders = Facet.define<(view: EditorView, from: number, to: number) => ContributedIntention[]>(); diff --git a/src/editor/core/model/attachment.ts b/src/editor/core/model/attachment.ts index 65f496e32..1a0701cfa 100644 --- a/src/editor/core/model/attachment.ts +++ b/src/editor/core/model/attachment.ts @@ -111,14 +111,7 @@ export async function attachDocument( if (model.ctx().readOnly || released || model.closed) return false; const slot = model.primarySlot(); if (slot?.view) return command(slot.view); - if (slot) - return command({ - state: slot.state, - dispatch: (tr) => { - slot.state = tr.state; - if (tr.docChanged) model.onLocal(tr.changes, slot); - }, - }); + if (slot) return command({ state: slot.state, dispatch: (tr) => slot.dispatchLocal(tr) }); return target.view ? command(target.view) : false; }; diff --git a/src/editor/core/model/document.test.ts b/src/editor/core/model/document.test.ts index 6f9993474..30b21b5b6 100644 --- a/src/editor/core/model/document.test.ts +++ b/src/editor/core/model/document.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { EditorState } from "@codemirror/state"; import { indentUnit, language as languageFacet } from "@codemirror/language"; -import { HEAVY_LIMIT, registerSaveParticipant, remoteChange } from "@/editor/api"; +import { HEAVY_LIMIT, registerSaveParticipant, remoteChange, shownAgain } from "@/editor/api"; import { createMemoryBackend, type MemoryBackend } from "../backend"; import { _resetLanguages } from "../cm/languages"; import { DocumentManager, ModifiedRanges, saveCleanupChanges, useDocMeta } from "./document"; @@ -142,9 +142,10 @@ describe("DocumentManager + slots", () => { const a = new EditorSlot(model, "t1", host); const b = new EditorSlot(model, "t2", host); let sawRemote = false; - const spy = vi.spyOn(a, "applyExternal").mockImplementation(function (this: EditorSlot, changes, annotations) { + const applyExternal = a.applyExternal.bind(a); + const spy = vi.spyOn(a, "applyExternal").mockImplementation((changes, annotations) => { sawRemote = annotations.some((x) => x.type === remoteChange); - this.state = this.state.update({ changes, annotations }).state; + applyExternal(changes, annotations); }); backend.patch(model.doc, [{ from: 3, to: 3, insert: "def" }]); expect(sawRemote).toBe(true); @@ -301,6 +302,51 @@ describe("DocumentManager + slots", () => { manager.stop(); } }); + + it("keeps a hidden tab's view and shows it again instead of building a new one", async () => { + const { backend, manager } = setup({ "/p/a.txt": "abc" }); + const model = await manager.acquire("/p/a.txt"); + const slot = new EditorSlot(model, "t1", host); + const shown: boolean[] = []; + slot.listeners.add((u) => shown.push(shownAgain(u))); + const view = slot.mount(document.body); + slot.unmount(); + expect(slot.view).toBeNull(); + expect(view.dom.isConnected).toBe(false); + // Changes while hidden go to the kept view. + backend.patch(model.doc, [{ from: 3, to: 3, insert: "def" }]); + expect(view.state.doc.toString()).toBe("abcdef"); + expect(slot.currentState()).toBe(view.state); + shown.length = 0; + expect(slot.mount(document.body)).toBe(view); + expect(view.dom.isConnected).toBe(true); + expect(shown).toContain(true); + slot.dispose(); + manager.stop(); + }); + + it("destroys the least recently hidden views beyond the kept ones, and kept views on dispose", async () => { + const { manager } = setup({ "/p/a.txt": "abc" }); + const model = await manager.acquire("/p/a.txt"); + const slots = Array.from({ length: 8 }, (_, i) => new EditorSlot(model, `t${i}`, host)); + const views = slots.map((slot) => { + const view = slot.mount(document.body); + slot.unmount(); + return view; + }); + const destroyed = views.map((view) => vi.spyOn(view, "destroy")); + slots.push(new EditorSlot(model, "t8", host)); + slots[8]!.mount(document.body); + slots[8]!.unmount(); + expect(destroyed[0]).toHaveBeenCalled(); + expect(destroyed[1]).not.toHaveBeenCalled(); + // A destroyed view leaves its state behind; showing the tab builds a new view on it. + expect(slots[0]!.mount(document.body)).not.toBe(views[0]); + slots[1]!.dispose(); + expect(destroyed[1]).toHaveBeenCalled(); + for (const slot of slots) slot.dispose(); + manager.stop(); + }); }); describe("save cleanup", () => { diff --git a/src/editor/core/model/slot.ts b/src/editor/core/model/slot.ts index 3f2467af1..77b180ae7 100644 --- a/src/editor/core/model/slot.ts +++ b/src/editor/core/model/slot.ts @@ -1,8 +1,10 @@ /** * An editor slot is one tab showing a text document in one pane. It owns the * tab's EditorState (selection, folds, undo history, scroll) and, while the - * tab is visible, an EditorView. Background tabs keep only their state; views - * are created on show and destroyed on hide, so many open tabs cost little. + * tab is visible, an EditorView. The most recently used tabs keep their view, + * detached, while hidden (`KEPT_VIEWS`): showing one again re-attaches it with + * its decorations, height map and plugin state; older background tabs keep + * only their state and get a new view when shown. * * All slots of a document share its DocModel: local edits flow * slot → model → backend and are mirrored into the other slots. @@ -23,6 +25,7 @@ import { _addEditor, _removeEditor, documentSynced, + viewShown, isExternalChange, transientChange, type EditorDocContext, @@ -53,11 +56,20 @@ export interface Reveal { center?: boolean; } +/** Hidden views kept for showing again, least recently hidden first. */ +const KEPT_VIEWS = 8; +const keptSlots: EditorSlot[] = []; + export class EditorSlot { - state: EditorState; + private stored: EditorState; private historyTarget: WorkspaceHistoryTarget; private detachHistory: () => void; + /** The view while the tab is visible. */ view: EditorView | null = null; + /** The view kept, detached from the document, while the tab is hidden. */ + private kept: EditorView | null = null; + /** The view's scroll offset, tracked so hiding it needn't read layout. */ + private scrollPos = { top: 0, left: 0 }; handle: TextEditorHandle | null = null; private scroll: StateEffect | null = null; private pendingReveal: Reveal | null = null; @@ -80,8 +92,8 @@ export class EditorSlot { path: model.path, state: () => this.currentState(), dispatch: (tr) => { - if (this.view) this.view.dispatch(tr); - else this.state = tr.state; + if (this.live) this.live.dispatch(tr); + else this.stored = tr.state; }, }; const source = init?.from; @@ -91,7 +103,7 @@ export class EditorSlot { .find((state) => state?.field(historyField, false)) : null; const history = secondary?.field(historyField, false); - this.state = EditorState.create({ + this.stored = EditorState.create({ doc: model.sync.localText, selection: source ? source.currentState().selection : undefined, extensions: [ @@ -121,8 +133,17 @@ export class EditorSlot { return this.model.ctx(); } + /** The visible or kept view: the one holding the tab's state. */ + private get live(): EditorView | null { + return this.view ?? this.kept; + } + + get state(): EditorState { + return this.live?.state ?? this.stored; + } + currentState(): EditorState { - return this.view?.state ?? this.state; + return this.state; } private slotExtensions() { @@ -137,6 +158,11 @@ export class EditorSlot { } }), Prec.highest(keymap.of([{ key: "Escape", run: () => this.host.onEscape?.(this) ?? false }])), + EditorView.domEventObservers({ + scroll: (_e, view) => { + if (view === this.view) this.scrollPos = { top: view.scrollDOM.scrollTop, left: view.scrollDOM.scrollLeft }; + }, + }), EditorView.domEventHandlers({ contextmenu: (e) => { this.host.onContextMenu(this, e); @@ -162,14 +188,28 @@ export class EditorSlot { mount(parent: HTMLElement): EditorView { if (this.view) return this.view; - this.syncContributions(); - const view = new EditorView({ - state: this.state, - parent, - dispatchTransactions: (trs, v) => this.onTransactions(trs, v), - }); - this.view = view; - if (this.scroll) view.dispatch({ effects: this.scroll }); + const kept = this.kept; + let view: EditorView; + if (kept) { + this.kept = null; + keptSlots.splice(keptSlots.indexOf(this), 1); + parent.appendChild(kept.dom); + // Detached, it lost its scroll offset. + kept.scrollDOM.scrollTop = this.scrollPos.top; + kept.scrollDOM.scrollLeft = this.scrollPos.left; + view = this.view = kept; + this.syncContributions(); + // Plugins that skipped work while it was hidden catch up. + view.dispatch({ effects: this.scroll ? [this.scroll, viewShown.of(null)] : viewShown.of(null) }); + } else { + this.syncContributions(); + view = this.view = new EditorView({ + state: this.stored, + parent, + dispatchTransactions: (trs, v) => this.onTransactions(trs, v), + }); + if (this.scroll) view.dispatch({ effects: this.scroll }); + } this.scroll = null; this.handle = { view, ctx: this.model.ctx() }; _addEditor(this.handle); @@ -181,22 +221,39 @@ export class EditorSlot { return view; } - unmount() { + /** Hides the view: keeps it detached for the most recent tabs, else (or `keep: false`) destroys it. */ + unmount(keep = true) { const view = this.view; if (!view) return; - this.savedTopLine = topLineOf(view); - this.state = view.state; - this.scroll = view.scrollSnapshot(); if (this.handle) _removeEditor(this.handle); this.handle = null; this.view = null; + this.kept = view; + if (!keep) return this.release(); + view.dom.remove(); + keptSlots.push(this); + if (keptSlots.length > KEPT_VIEWS) keptSlots.shift()!.release(); + } + + /** Destroys the kept view, keeping its state. */ + private release() { + const view = this.kept; + if (!view) return; + this.kept = null; + const i = keptSlots.indexOf(this); + if (i >= 0) keptSlots.splice(i, 1); + const top = topLineAt(view, this.scrollPos.top); + this.savedTopLine = top; + this.scroll = EditorView.scrollIntoView(view.state.doc.line(top + 1).from, { y: "start" }); + this.stored = view.state; view.destroy(); } dispose() { if (this.disposed) return; this.detachHistory(); - this.unmount(); + this.unmount(false); + this.release(); this.disposed = true; this.model.removeSlot(this); } @@ -237,13 +294,13 @@ export class EditorSlot { } /** Applies a transaction as if the user made it (synced, undoable). */ - dispatchLocal(spec: TransactionSpec) { - if (this.view) { - this.view.dispatch(spec); + dispatchLocal(spec: TransactionSpec | Transaction) { + if (this.live) { + this.live.dispatch(spec); return; } - const tr = this.state.update(spec); - this.state = tr.state; + const tr = "startState" in spec ? spec : this.stored.update(spec); + this.stored = tr.state; if (tr.docChanged) this.model.onLocal(tr.changes, this); if (tr.docChanged && tr.isUserEvent("format.reformat")) this.model.redetectIndent(); } @@ -251,15 +308,15 @@ export class EditorSlot { /** Applies changes that are already in the model (other slot / backend). */ applyExternal(changes: ChangeSet, annotations: Annotation[]) { const spec: TransactionSpec = { changes, annotations }; - if (this.view) this.view.dispatch(spec); - else this.state = this.state.update(spec).state; + if (this.live) this.live.dispatch(spec); + else this.stored = this.stored.update(spec).state; } /** Applies effects (reconfiguration) to the view or the stored state. */ reconfigure(effects: StateEffect[]) { if (!effects.length) return; - if (this.view) this.view.dispatch({ effects }); - else this.state = this.state.update({ effects }).state; + if (this.live) this.live.dispatch({ effects }); + else this.stored = this.stored.update({ effects }).state; if (this.handle) this.handle.ctx = this.model.ctx(); } @@ -278,7 +335,7 @@ export class EditorSlot { const anchor = Math.min(Math.max(0, r.anchor), len); const head = Math.min(Math.max(0, r.head ?? anchor), len); if (!this.view) { - this.state = this.state.update({ selection: EditorSelection.single(anchor, head) }).state; + this.dispatchState({ selection: EditorSelection.single(anchor, head) }); this.pendingReveal = r; return; } @@ -294,7 +351,11 @@ export class EditorSlot { /** Caret and first visible line, for the persisted editor session. */ viewState(): { anchor: number; head: number; topLine?: number } { const main = this.currentState().selection.main; - const topLine = this.view ? topLineOf(this.view) : this.savedTopLine; + const topLine = this.view + ? topLineOf(this.view) + : this.kept + ? topLineAt(this.kept, this.scrollPos.top) + : this.savedTopLine; return { anchor: main.anchor, head: main.head, ...(topLine !== undefined ? { topLine } : {}) }; } @@ -313,13 +374,19 @@ export class EditorSlot { this.view.dispatch({ selection, effects: scroll ? [scroll] : [] }); return; } - this.state = this.state.update({ selection }).state; + this.dispatchState({ selection }); if (scroll) { this.scroll = scroll; this.savedTopLine = v.topLine; } } + /** Updates the state of a hidden tab (its kept view, if any). */ + private dispatchState(spec: TransactionSpec) { + if (this.kept) this.kept.dispatch(spec); + else this.stored = this.stored.update(spec).state; + } + caret(): { line: number; col: number; offset: number } { const s = this.currentState(); const head = s.selection.main.head; @@ -329,6 +396,10 @@ export class EditorSlot { } function topLineOf(view: EditorView): number { - const block = view.lineBlockAtHeight(Math.max(0, view.scrollDOM.scrollTop)); + return topLineAt(view, view.scrollDOM.scrollTop); +} + +function topLineAt(view: EditorView, scrollTop: number): number { + const block = view.lineBlockAtHeight(Math.max(0, scrollTop)); return view.state.doc.lineAt(block.from).number - 1; } diff --git a/src/editor/core/model/workspaceHistory.test.ts b/src/editor/core/model/workspaceHistory.test.ts index fc9e29821..0cd7a0803 100644 --- a/src/editor/core/model/workspaceHistory.test.ts +++ b/src/editor/core/model/workspaceHistory.test.ts @@ -62,9 +62,7 @@ async function rename() { function historyStep(slot: EditorSlot, forward = false) { return (forward ? redo : undo)({ state: slot.currentState(), - dispatch: (tr) => { - slot.state = tr.state; - }, + dispatch: (tr) => slot.dispatchLocal(tr), }); } diff --git a/src/editor/lsp/codeLens.ts b/src/editor/lsp/codeLens.ts index b8051deac..34e7b1c5b 100644 --- a/src/editor/lsp/codeLens.ts +++ b/src/editor/lsp/codeLens.ts @@ -8,7 +8,7 @@ import { Channel } from "@tauri-apps/api/core"; import { EditorSelection, MapMode, StateEffect, StateField, type Extension, type Text } from "@codemirror/state"; import { EditorView, ViewPlugin, type ViewUpdate } from "@codemirror/view"; -import { codeLenses, type CodeLens } from "@/editor/api"; +import { codeLenses, shownAgain, type CodeLens } from "@/editor/api"; import { executeAction } from "@/platform/actions"; import { commands, nextRequestId, type OutlineItem, type RequestId, type SymbolKind } from "@/platform/ipc"; import { useProject } from "@/platform/project"; @@ -119,6 +119,7 @@ class UsagesLensPlugin { private offReady: () => void; private offSettled: () => void; private offBlame: () => void; + private skipped = false; constructor(private view: EditorView) { this.refresh(); @@ -137,7 +138,7 @@ class UsagesLensPlugin { } update(u: ViewUpdate) { - if (u.docChanged || u.viewportChanged) this.refresh(); + if (u.docChanged || u.viewportChanged || (this.skipped && shownAgain(u))) this.refresh(); } private cancel() { @@ -153,7 +154,8 @@ class UsagesLensPlugin { private async run() { const view = this.view; - if (!view.dom.isConnected) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; const doc = docOf(view); if (!doc) return; const gen = this.generation; diff --git a/src/editor/lsp/diagnostics.test.ts b/src/editor/lsp/diagnostics.test.ts index 285602944..0b9adc8f3 100644 --- a/src/editor/lsp/diagnostics.test.ts +++ b/src/editor/lsp/diagnostics.test.ts @@ -1,7 +1,12 @@ import { describe, expect, it, vi } from "vitest"; -import { Text } from "@codemirror/state"; +import { EditorState, Text } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { viewShown, type EditorDocContext } from "@/editor/api"; +import { lspDoc } from "./lsp"; import type { Diagnostic } from "@/bindings"; import { + diagnosticsAt, + diagnosticsExtension, diagnosticsFor, installDiagnosticsEvents, setFileDiagnostics, @@ -84,3 +89,25 @@ describe("toLintDiagnostics", () => { expect(toLintDiagnostics(doc, [d(2, 1, 1)])[0]).toMatchObject({ from: doc.length - 1, to: doc.length }); }); }); + +describe("editor diagnostics in a view", () => { + it("skips a hidden view and catches up when it is shown again", async () => { + const view = new EditorView({ + state: EditorState.create({ + doc: "let x = 1;", + extensions: [lspDoc.of({ doc: 1, path: "/p/hidden.rs" } as EditorDocContext), diagnosticsExtension()], + }), + parent: document.body, + }); + view.dom.remove(); + setFileDiagnostics("/p/hidden.rs", "rust-analyzer", [d(0, 4, 5)]); + await Promise.resolve(); + expect(diagnosticsAt(view, 4)).toHaveLength(0); + document.body.appendChild(view.dom); + view.dispatch({ effects: viewShown.of(null) }); + await Promise.resolve(); + expect(diagnosticsAt(view, 4)).toHaveLength(1); + view.destroy(); + setFileDiagnostics("/p/hidden.rs", "rust-analyzer", []); + }); +}); diff --git a/src/editor/lsp/diagnostics.ts b/src/editor/lsp/diagnostics.ts index 917ec8023..751b401f9 100644 --- a/src/editor/lsp/diagnostics.ts +++ b/src/editor/lsp/diagnostics.ts @@ -6,13 +6,13 @@ */ import type { Text } from "@codemirror/state"; import { EditorSelection, StateEffect, StateField } from "@codemirror/state"; -import { EditorView, ViewPlugin } from "@codemirror/view"; +import { EditorView, ViewPlugin, type ViewUpdate } from "@codemirror/view"; import { forEachDiagnostic, linter, setDiagnostics, type Diagnostic as LintDiagnostic } from "@codemirror/lint"; import type { Diagnostic, Severity } from "@/bindings"; import { commands, ideEventsReady, isTauri, onIdeEvent } from "@/platform/ipc"; import { useProject } from "@/platform/project"; import { publishLocalDiagnostics } from "@/platform/localDiagnostics"; -import { intentionsChanged, scrollbarMarks, type ScrollbarMark } from "@/editor/api"; +import { intentionsChanged, scrollbarMarks, shownAgain, type ScrollbarMark } from "@/editor/api"; import { rangeToOffsets } from "./positions"; import { lspDoc } from "./lsp"; import { showHint } from "./hint"; @@ -190,6 +190,7 @@ const diagnosticsSync = ViewPlugin.fromClass( class { private unsubscribe: () => void; private pending = false; + private skipped = false; constructor(private view: EditorView) { const path = view.state.facet(lspDoc)?.path; this.unsubscribe = path ? subscribeDiagnostics(path, () => this.schedule()) : () => {}; @@ -201,7 +202,8 @@ const diagnosticsSync = ViewPlugin.fromClass( queueMicrotask(() => { this.pending = false; const path = this.view.state.facet(lspDoc)?.path; - if (!path || !this.view.dom.isConnected) return; + this.skipped = !this.view.dom.isConnected; + if (!path || this.skipped) return; const lint = toLintDiagnostics(this.view.state.doc, diagnosticsFor(path)); const tr = setDiagnostics(this.view.state, lint); this.view.dispatch({ @@ -210,6 +212,9 @@ const diagnosticsSync = ViewPlugin.fromClass( }); }); } + update(u: ViewUpdate) { + if (this.skipped && shownAgain(u)) this.schedule(); + } destroy() { this.unsubscribe(); } diff --git a/src/editor/lsp/inlay.ts b/src/editor/lsp/inlay.ts index 49c645c92..9c0c57ffd 100644 --- a/src/editor/lsp/inlay.ts +++ b/src/editor/lsp/inlay.ts @@ -5,6 +5,7 @@ * offers to disable hints of that kind. */ import { StateEffect, StateField, type Extension } from "@codemirror/state"; +import { shownAgain } from "@/editor/api"; import { Decoration, EditorView, ViewPlugin, WidgetType, type DecorationSet, type ViewUpdate } from "@codemirror/view"; import { call, commands, isTauri, type InlayHint } from "@/platform/ipc"; import { executeAction } from "@/platform/actions"; @@ -162,6 +163,7 @@ const inlayPlugin = ViewPlugin.fromClass( private unsubscribe: () => void; private offReady: () => void; private offModifier: () => void; + private skipped = false; constructor(private view: EditorView) { this.refresh(); view.contentDOM.classList.toggle("cm-lsp-navigation-modifier", navigationModifierHeld()); @@ -178,11 +180,12 @@ const inlayPlugin = ViewPlugin.fromClass( this.offReady = onLspRefresh("inlayHints", () => this.refresh()); } update(u: ViewUpdate) { - if (u.docChanged || u.viewportChanged) this.refresh(); + if (u.docChanged || u.viewportChanged || (this.skipped && shownAgain(u))) this.refresh(); } async run() { const view = this.view; - if (!view.dom.isConnected) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; const doc = docOf(view); if (!doc) return; if (!enabled(doc.language) || inlayBackoff.active(doc.doc)) { diff --git a/src/editor/lsp/serverCodeLens.ts b/src/editor/lsp/serverCodeLens.ts index c7ed3d7c7..a69533ab0 100644 --- a/src/editor/lsp/serverCodeLens.ts +++ b/src/editor/lsp/serverCodeLens.ts @@ -1,7 +1,7 @@ /** Language-server and plugin lenses in the shared editor code-vision renderer. */ import { StateEffect, StateField, type Extension } from "@codemirror/state"; import { ViewPlugin, type ViewUpdate } from "@codemirror/view"; -import { codeLenses, whenSynced, type CodeLens, type EditorDocContext } from "@/editor/api"; +import { codeLenses, shownAgain, whenSynced, type CodeLens, type EditorDocContext } from "@/editor/api"; import { call, commands } from "@/platform/ipc"; import { onNavigationProvidersChanged } from "@/editor/navigationProviders"; import { posToOffset } from "./positions"; @@ -24,6 +24,7 @@ export function serverCodeLenses(ctx: EditorDocContext): Extension { new (class { timer: ReturnType | undefined; generation = 0; + skipped = false; off: () => void; constructor() { const offs = [ @@ -40,7 +41,7 @@ export function serverCodeLenses(ctx: EditorDocContext): Extension { this.timer = setTimeout(() => void this.run(), 400); } update(u: ViewUpdate) { - if (u.docChanged) this.schedule(); + if (u.docChanged || (this.skipped && shownAgain(u))) this.schedule(); } async run() { const gen = this.generation; @@ -48,7 +49,9 @@ export function serverCodeLenses(ctx: EditorDocContext): Extension { try { await whenSynced(view); const result = await call(commands.lspCodeLens(ctx.doc)); - if (gen !== this.generation || text !== view.state.doc || !view.dom.isConnected) return; + if (gen !== this.generation || text !== view.state.doc) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; view.dispatch({ effects: set.of( result.flatMap((l) => diff --git a/src/editor/navigation/implementationGutter.ts b/src/editor/navigation/implementationGutter.ts index 3a99aa62c..6455c01d4 100644 --- a/src/editor/navigation/implementationGutter.ts +++ b/src/editor/navigation/implementationGutter.ts @@ -15,7 +15,7 @@ */ import { RangeSet, StateEffect, StateField, type Extension, type Text } from "@codemirror/state"; import { EditorView, GutterMarker, ViewPlugin, gutter, type ViewUpdate } from "@codemirror/view"; -import type { EditorDocContext } from "@/editor/api"; +import { shownAgain, type EditorDocContext } from "@/editor/api"; import { docOf, debounce, isUnavailable, lspRequest } from "@/editor/lsp/lsp"; import { featureBackoff, onServerSettled } from "@/editor/lsp/availability"; import { readManyFileLines, withDocument } from "@/editor/lsp/files"; @@ -295,6 +295,7 @@ function markerSet(placed: readonly PlacedMarker[]): RangeSet { class MarkersPlugin { private refresh = debounce(() => void this.run(), DEBOUNCE_MS); private generation = 0; + private skipped = false; private request: RequestId | null = null; private offSettings: () => void; @@ -316,7 +317,7 @@ class MarkersPlugin { // Other documents' markers may depend on this one. changes++; this.refresh(); - } else if (u.viewportChanged) this.refresh(); + } else if (u.viewportChanged || (this.skipped && shownAgain(u))) this.refresh(); // Other files may have changed meanwhile (a new implementation). else if (u.focusChanged && u.view.hasFocus) { const ctx = docOf(u.view); @@ -337,7 +338,8 @@ class MarkersPlugin { private async run() { const view = this.view; - if (!view.dom.isConnected) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; const ctx = docOf(view); if (!ctx) return; const gen = ++this.generation; diff --git a/src/features/color-preview/pluginColors.ts b/src/features/color-preview/pluginColors.ts index afc1f4dcc..87f12db89 100644 --- a/src/features/color-preview/pluginColors.ts +++ b/src/features/color-preview/pluginColors.ts @@ -1,7 +1,7 @@ /** Plugin colors use the same native picker and provider-supplied replacement strings. */ import { RangeSet, StateEffect, StateField, type Extension } from "@codemirror/state"; import { GutterMarker, ViewPlugin, gutter, type EditorView, type ViewUpdate } from "@codemirror/view"; -import { type EditorDocContext } from "@/editor/api"; +import { shownAgain, type EditorDocContext } from "@/editor/api"; import { rangeToOffsets } from "@/editor/lsp/positions"; import { pluginVisuals } from "@/editor/lsp/pluginVisuals"; import { onNavigationProvidersChanged } from "@/editor/navigationProviders"; @@ -118,6 +118,7 @@ export function pluginColorPreview(ctx: EditorDocContext): Extension { new (class { timer: ReturnType | undefined; generation = 0; + skipped = false; off: () => void; constructor() { this.off = onNavigationProvidersChanged(() => this.schedule()); @@ -129,15 +130,16 @@ export function pluginColorPreview(ctx: EditorDocContext): Extension { this.timer = setTimeout(() => void this.run(), 300); } update(u: ViewUpdate) { - if (u.docChanged) this.schedule(); + if (u.docChanged || (this.skipped && shownAgain(u))) this.schedule(); } async run() { const gen = this.generation; const text = view.state.doc; try { const r = await pluginVisuals(view, ctx); - if (gen === this.generation && view.state.doc === text && view.dom.isConnected) - view.dispatch({ effects: setColors.of(r.colors) }); + if (gen !== this.generation || view.state.doc !== text) return; + this.skipped = !view.dom.isConnected; + if (!this.skipped) view.dispatch({ effects: setColors.of(r.colors) }); } catch { if (gen === this.generation && view.dom.isConnected) view.dispatch({ effects: setColors.of([]) }); } diff --git a/src/features/plugins/languageVisuals.ts b/src/features/plugins/languageVisuals.ts index d14440932..853dd522c 100644 --- a/src/features/plugins/languageVisuals.ts +++ b/src/features/plugins/languageVisuals.ts @@ -19,7 +19,7 @@ import { } from "@codemirror/view"; import { highlightingFor } from "@codemirror/language"; import { highlightTree } from "@lezer/highlight"; -import { type EditorDocContext } from "@/editor/api"; +import { shownAgain, type EditorDocContext } from "@/editor/api"; import { rangeToOffsets } from "@/editor/lsp/positions"; import { pluginVisuals } from "@/editor/lsp/pluginVisuals"; import { findLanguageDescription, languageInfo, loadSupport } from "@/editor/core/cm/languages"; @@ -123,6 +123,7 @@ export function pluginLanguageVisuals(ctx: EditorDocContext): Extension { timer: ReturnType | undefined; generation = 0; shown = false; + skipped = false; off: () => void; constructor() { const a = onNavigationProvidersChanged(() => this.schedule(true)); @@ -147,7 +148,7 @@ export function pluginLanguageVisuals(ctx: EditorDocContext): Extension { this.timer = setTimeout(() => void this.run(), 300); } update(u: ViewUpdate) { - if (u.docChanged) this.schedule(); + if (u.docChanged || (this.skipped && shownAgain(u))) this.schedule(); } async run() { const generation = this.generation; @@ -163,7 +164,9 @@ export function pluginLanguageVisuals(ctx: EditorDocContext): Extension { marker: new ActionMarker(plugin, { ...a, icon: await resolveMarkerIcon(plugin, a.icon) }, ctx), })), ); - if (generation !== this.generation || view.state.doc !== text || !view.dom.isConnected) return; + if (generation !== this.generation || view.state.doc !== text) return; + this.skipped = !view.dom.isConnected; + if (this.skipped) return; this.shown = !!placed.length; view.dispatch({ effects: [ diff --git a/src/features/plugins/lineMarkerGutter.ts b/src/features/plugins/lineMarkerGutter.ts index 6f611ed5c..0ff26e462 100644 --- a/src/features/plugins/lineMarkerGutter.ts +++ b/src/features/plugins/lineMarkerGutter.ts @@ -6,7 +6,7 @@ */ import { Compartment, RangeSet, StateEffect, StateField, type Extension, type Text } from "@codemirror/state"; import { EditorView, GutterMarker, ViewPlugin, gutter, type ViewUpdate } from "@codemirror/view"; -import type { EditorDocContext } from "@/editor/api"; +import { shownAgain, type EditorDocContext } from "@/editor/api"; import { navigationButton } from "./navigationButton"; import { pluginsBackend } from "./backend"; import { commands, type LineMarkerInfo } from "@/platform/ipc"; @@ -79,6 +79,7 @@ class PluginMarker extends GutterMarker { class MarkersPlugin { private timer: ReturnType | null = null; private generation = 0; + private skipped = false; private shown = false; private off: () => void; @@ -106,7 +107,7 @@ class MarkersPlugin { } update(u: ViewUpdate) { - if (u.docChanged) this.schedule(); + if (u.docChanged || (this.skipped && shownAgain(u))) this.schedule(); } private schedule() { @@ -128,7 +129,9 @@ class MarkersPlugin { icon: await resolveMarkerIcon(marker.pluginId, marker.icon), })), ); - if (gen !== this.generation || this.view.state.doc !== text || !this.view.dom.isConnected) return; + if (gen !== this.generation || this.view.state.doc !== text) return; + this.skipped = !this.view.dom.isConnected; + if (this.skipped) return; const placed = placeMarkers(text, resolved); if (!placed.length && !this.view.state.field(markersField).length) return; const effects: StateEffect[] = [setMarkers.of(placed)]; diff --git a/src/features/search/__tests__/dialogs.test.tsx b/src/features/search/__tests__/dialogs.test.tsx index 8b5f528ad..ad17e6beb 100644 --- a/src/features/search/__tests__/dialogs.test.tsx +++ b/src/features/search/__tests__/dialogs.test.tsx @@ -479,7 +479,7 @@ describe("Recent Files multi-selection", () => { describe("Switcher", () => { afterEach(() => vi.restoreAllMocks()); - it("opens the selection when Ctrl is released, once the popup gave focus back, in the same task", async () => { + it("opens the selection when Ctrl is released, once the popup closed, in the same task", async () => { const tab = (key: string, lastActive: number) => ({ input: { kind: "agent-session", key }, title: key, @@ -508,7 +508,9 @@ describe("Switcher", () => { await tick(); expect(screen.getByRole("dialog", { name: "Switcher" })).toBeInTheDocument(); act(() => void fireEvent.keyUp(window, { key: "Control" })); - expect(seen).toEqual([{ key: "task-a", popup: false, focus: editor }]); + // The editor opened takes focus itself: the one left behind doesn't get it back first. + expect(seen).toEqual([{ key: "task-a", popup: false, focus: document.body }]); + expect(editor).not.toHaveFocus(); }); }); diff --git a/src/features/search/recent/RecentPopups.tsx b/src/features/search/recent/RecentPopups.tsx index 68013c4a7..47e37096a 100644 --- a/src/features/search/recent/RecentPopups.tsx +++ b/src/features/search/recent/RecentPopups.tsx @@ -62,11 +62,22 @@ export function RecentPopupsOverlay() { const NAV_KEYS = ["ArrowDown", "ArrowUp", "PageDown", "PageUp"]; +let handingFocus = false; +// Asked when the dialog's effects are cleaned up, which may come after `fn` already ran. +const keepFocus = () => { + const keep = !handingFocus; + handingFocus = false; + return keep; +}; + /** * Runs `fn` once the dialog has unmounted and given focus back, in the same frame: the * popup closing and what it opens paint together, without a frame of the old editor between. + * `takesFocus`: what `fn` opens focuses itself, so focus isn't given back first (focusing the + * editor left behind would lay out the window once more). */ -const afterClose = (fn: () => void) => { +const afterClose = (fn: () => void, takesFocus = false) => { + handingFocus = takesFocus; flushSync(hideRecentPopup); fn(); }; @@ -143,10 +154,12 @@ function RecentFilesDialog() { const openFile = (item: RecentFileItem | undefined, side = false) => { if (!item) return; const editor = item.editor; - afterClose(() => - editor - ? void maybeEditorService()?.openInput(editor.input, { side, focus: true }) - : void openLocation(item.path, null, { side }), + afterClose( + () => + editor + ? void maybeEditorService()?.openInput(editor.input, { side, focus: true }) + : void openLocation(item.path, null, { side }), + !!editor, ); }; @@ -282,7 +295,14 @@ function RecentFilesDialog() { const title = switcher ? "Switcher" : editedOnly ? "Recently Edited Files" : "Recent Files"; const footerPath = (selected?.editor ? selected.rel : selected?.path) ?? (selectedTool ? "" : (root ?? "")); return ( - +
{title} diff --git a/src/ui/Dialog.tsx b/src/ui/Dialog.tsx index 3e6d9c119..03e757e2a 100644 --- a/src/ui/Dialog.tsx +++ b/src/ui/Dialog.tsx @@ -135,6 +135,11 @@ export interface DialogProps { dismissOnBackdrop?: boolean; /** Close when the app window loses focus (default: popups only; never modal dialogs). */ closeOnDeactivate?: boolean; + /** + * Asked on close: give focus back to what had it before the dialog (default: always). False + * when what the dialog opens takes focus itself. + */ + restoreFocus?: () => boolean; /** @deprecated Dialogs never dim the window behind them; kept for call sites. */ transparentBackdrop?: boolean; /** @deprecated Every centred dialog has window chrome; kept for call sites. */ @@ -219,6 +224,7 @@ function DialogInner({ className, dismissOnBackdrop, closeOnDeactivate, + restoreFocus, platform: platformOverride, icon, ...aria @@ -231,9 +237,11 @@ function DialogInner({ const popup = position === "top" || (title == null && !icon); const onCloseRef = useRef(onClose); const buttonsRef = useRef(buttons); + const restoreFocusRef = useRef(restoreFocus); useLayoutEffect(() => { onCloseRef.current = onClose; buttonsRef.current = buttons; + restoreFocusRef.current = restoreFocus; }); const [offset, setOffset] = useState({ x: 0, y: 0 }); @@ -299,7 +307,7 @@ function DialogInner({ return () => { document.removeEventListener("keydown", onKey); layer.release(); - if (previous?.isConnected) previous.focus({ preventScroll: true }); + if (previous?.isConnected && restoreFocusRef.current?.() !== false) previous.focus({ preventScroll: true }); }; }, []); diff --git a/src/ui/Tabs.tsx b/src/ui/Tabs.tsx index 6f1f5bddb..2ad4afdbd 100644 --- a/src/ui/Tabs.tsx +++ b/src/ui/Tabs.tsx @@ -215,18 +215,43 @@ export function Tabs({ }); }, [dndType]); + // Tab slots wholly in view as last observed, which needs no layout read: switching to one of + // them doesn't measure the strip (a forced layout of the window, mid-switch). + const inView = useRef(new Set()); // Bring a tab's whole slot (label + close button) into view. Narrowing the strip clips // the active tab's close button (#1740), and clicking the already-active tab doesn't // change `activeId`, so the strip must also reveal on clicks and resizes. const revealTab = useCallback( - (id: string | null) => { + (id: string | null, always = false) => { const scroller = scrollRef.current; const tab = id ? tabRefs.current.get(id)?.parentElement : null; - if (scroller && tab) (vertical ? revealVertically : revealHorizontally)(scroller, tab); + if (!scroller || !tab || (!always && inView.current.has(tab))) return; + (vertical ? revealVertically : revealHorizontally)(scroller, tab); }, [vertical], ); + const tabIds = items.map((it) => it.id).join("\n"); + useEffect(() => { + const root = scrollRef.current; + if (!root || typeof IntersectionObserver === "undefined") return; + const seen = inView.current; + const io = new IntersectionObserver( + (entries) => { + for (const e of entries) { + if (e.intersectionRatio >= 1) seen.add(e.target); + else seen.delete(e.target); + } + }, + { root, threshold: 1 }, + ); + for (const tab of tabRefs.current.values()) if (tab.parentElement) io.observe(tab.parentElement); + return () => { + io.disconnect(); + seen.clear(); + }; + }, [tabIds]); + useLayoutEffect(() => { const el = scrollRef.current; if (!el) return; @@ -235,7 +260,7 @@ export function Tabs({ setOverflow(!vertical && el.scrollWidth > el.clientWidth + 1); // A resize (e.g. the editor area narrowing) must scroll the active tab back // into view instead of leaving its close button clipped until a tab switch. - revealTab(latest.current.activeId); + revealTab(latest.current.activeId, true); }; measure(); if (typeof ResizeObserver === "undefined") return; diff --git a/src/workbench/focus.test.ts b/src/workbench/focus.test.ts index 965176321..62ed4eae1 100644 --- a/src/workbench/focus.test.ts +++ b/src/workbench/focus.test.ts @@ -38,6 +38,21 @@ describe("focus after popups", () => { dispose(); }); + it("leaves the caret to the next editor when focus moves between editors", () => { + document.body.innerHTML = `
first
second
`; + const [first, second] = document.querySelectorAll(".cm-content"); + const dispose = installFocusTracking(() => {}); + first!.focus(); + document.getSelection()!.collapse(first!.firstChild, 3); + let caretLeft: Node | null = null; + const probe = () => (caretLeft = document.getSelection()!.anchorNode); + document.addEventListener("focusout", probe); + second!.focus(); + document.removeEventListener("focusout", probe); + expect(caretLeft).toBe(first!.firstChild); + dispose(); + }); + it("drops the editor's caret when focus moves to a menu, so focusing the editor again cannot scroll to it", () => { document.body.innerHTML = `
first line
`; const editor = document.querySelector(".cm-content")!; diff --git a/src/workbench/focus.ts b/src/workbench/focus.ts index 03b2426a7..54e133f1b 100644 --- a/src/workbench/focus.ts +++ b/src/workbench/focus.ts @@ -39,6 +39,9 @@ function dropEditorCaret(e: FocusEvent) { // The window lost focus and keeps the editor focused for its return. if (document.activeElement === editor) return; if (e.relatedTarget instanceof Node && editor.contains(e.relatedTarget)) return; + // Another editor takes the DOM selection itself; a hidden (detached) editor has none left. + // Dropping it would only lay out the window once more while switching editors. + if (!editor.isConnected || (e.relatedTarget instanceof Element && e.relatedTarget.matches(".cm-content"))) return; const selection = document.getSelection(); if (selection?.anchorNode && editor.contains(selection.anchorNode)) selection.removeAllRanges(); }