diff --git a/src/features/search/__tests__/findPopup.test.tsx b/src/features/search/__tests__/findPopup.test.tsx index 0c6593ea3..9d10f9f6b 100644 --- a/src/features/search/__tests__/findPopup.test.tsx +++ b/src/features/search/__tests__/findPopup.test.tsx @@ -1,15 +1,22 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { act, fireEvent, render, screen, within } from "@testing-library/react"; +import type { FileMatches, SearchSummary } from "@/platform/ipc"; import type { ReplaceRequest } from "../find/replaceFlow"; const replaced: ReplaceRequest[] = []; const confirms: unknown[] = []; +let validation: Promise | null = null; +let search: ((onFile: (file: FileMatches) => void) => Promise) | null = null; +let confirmation: Promise | null = null; vi.mock("../find/windowStore", async (orig) => ({ ...(await orig()), ipcFindDeps: () => ({ - validate: async () => {}, - start: async (_id: number, _q: unknown, onFile: (f: unknown) => void) => { + validate: async () => { + await validation; + }, + start: async (_id: number, _q: unknown, onFile: (f: FileMatches) => void) => { + if (search) return search(onFile); onFile({ path: "/p/a.kt", relPath: "a.kt", @@ -44,7 +51,7 @@ vi.mock("@/workbench/messages", async (orig) => ({ ...(await orig()), confirm: async (opts: unknown) => { confirms.push(opts); - return true; + return confirmation ?? true; }, })); @@ -63,7 +70,22 @@ beforeEach(() => { replaced.length = 0; confirms.length = 0; localStorage.clear(); - useFindPopup.setState({ pinned: false, maskEnabled: false, preserveCase: false, context: "any", replacement: "" }); + validation = null; + search = null; + confirmation = null; + useFindPopup.setState({ + pattern: "", + caseSensitive: false, + wholeWord: false, + isRegex: false, + scopeTab: "project", + mask: "", + pinned: false, + maskEnabled: false, + preserveCase: false, + context: "any", + replacement: "", + }); }); afterEach(() => { @@ -163,3 +185,153 @@ describe("Find in Files popup", () => { expect(findHistory("find")[0]).toBe("order"); }); }); + +describe("outdated popup results", () => { + const results = () => screen.getByRole("listbox", { name: "Matches" }).parentElement!; + const rowText = () => + within(screen.getByRole("listbox", { name: "Matches" })) + .getAllByRole("option") + .map((r) => r.textContent); + + it.each([false, true])( + "marks retained rows immediately in replace=%s and waits for the first new file", + async (replace) => { + render(); + act(() => showFindPopup(replace, { query: "order" })); + await settle(); + if (replace) fireEvent.change(screen.getByLabelText("Replace with"), { target: { value: "item" } }); + const oldRows = within(screen.getByRole("listbox", { name: "Matches" })).getAllByRole("option"); + let validate!: () => void; + validation = new Promise((r) => { + validate = r; + }); + let onFile!: (file: FileMatches) => void; + let finish!: (summary: SearchSummary | null) => void; + search = (callback) => { + onFile = callback; + return new Promise((r) => { + finish = r; + }); + }; + fireEvent.change(field(), { target: { value: "fresh" } }); + expect(results()).toHaveAttribute("data-stale", "true"); + expect(screen.getByText("Updating results…")).toBeInTheDocument(); + expect(within(results()).getAllByRole("option")).toEqual(oldRows); + expect(results().querySelector("mark, del, ins")).toBeNull(); + expect(screen.queryByText("3 matches in 2 files")).not.toBeInTheDocument(); + await settle(); + expect(results()).toHaveAttribute("data-stale", "true"); + expect(rowText()).toHaveLength(3); + await act(async () => validate()); + expect(results()).toHaveAttribute("data-stale", "true"); + expect(rowText()).toHaveLength(3); + if (replace) expect(screen.getByRole("button", { name: "Replace" })).toBeDisabled(); + await act(async () => + onFile({ + path: "/p/fresh.kt", + relPath: "fresh.kt", + matches: [{ line: 0, text: "fresh", ranges: [[0, 5]], textOffset: 0 }], + }), + ); + expect(results()).not.toHaveAttribute("data-stale"); + expect(rowText()).toHaveLength(1); + expect(screen.getByText("1 match in 1 file")).toBeInTheDocument(); + expect(results().querySelector(replace ? "ins" : "mark")).not.toBeNull(); + if (replace) expect(screen.getByRole("button", { name: "Replace" })).toBeEnabled(); + await act(async () => finish(null)); + }, + ); + + it.each(["Replace", "Replace All in This File", "Replace All", "Enter", "All shortcut", "All mnemonic"])( + "blocks %s during debounce and validation", + async (action) => { + render(); + act(() => showFindPopup(true, { query: "order" })); + await settle(); + validation = new Promise(() => {}); + fireEvent.change(field(), { target: { value: "ord" } }); + const invoke = () => { + if (action === "Enter") fireEvent.keyDown(field(), { key: "Enter" }); + else if (action === "All shortcut") fireEvent.keyDown(field(), { key: "Enter", altKey: true, shiftKey: true }); + else if (action === "All mnemonic") fireEvent.keyDown(field(), { key: "a", code: "KeyA", altKey: true }); + else { + const button = screen.getByRole("button", { name: action }); + expect(button).toBeDisabled(); + fireEvent.click(button); + } + }; + invoke(); + await settle(); + invoke(); + await act(async () => {}); + expect(replaced).toHaveLength(0); + expect(confirms).toHaveLength(0); + }, + ); + + it.each([ + { caseSensitive: true }, + { wholeWord: true }, + { isRegex: true }, + { maskEnabled: true, mask: "*.ts" }, + { scopeTab: "directory" as const, directory: "/other" }, + { context: "inComments" as const }, + ])("invalidates when search options change: %j", async (patch) => { + render(); + act(() => showFindPopup(true, { query: "order" })); + await settle(); + act(() => useFindPopup.setState(patch)); + expect(results()).toHaveAttribute("data-stale", "true"); + expect(results().querySelector("mark, del, ins")).toBeNull(); + expect(screen.getByRole("button", { name: "Replace" })).toBeDisabled(); + }); + + it("clears retained rows and the count when the new search finds nothing", async () => { + render(); + act(() => showFindPopup(true, { query: "order" })); + await settle(); + let finish!: (summary: SearchSummary | null) => void; + search = () => + new Promise((r) => { + finish = r; + }); + fireEvent.change(field(), { target: { value: "absent" } }); + await settle(); + expect(results()).toHaveAttribute("data-stale", "true"); + expect(rowText()).toHaveLength(3); + const box = results(); + await act(async () => finish(null)); + expect(box).not.toHaveAttribute("data-stale"); + expect(screen.getByText("No matches")).toBeInTheDocument(); + expect(screen.getByText("Nothing found")).toBeInTheDocument(); + expect(within(box).queryAllByRole("option")).toHaveLength(0); + expect(screen.getByRole("button", { name: "Replace" })).toBeDisabled(); + }); + + it.each([false, true])("keeps opening an outdated result available in replace=%s", async (replace) => { + render(); + act(() => showFindPopup(replace, { query: "order" })); + await settle(); + fireEvent.change(field(), { target: { value: "absent" } }); + if (replace) act(() => void runFindPopupCommand("openResult")); + else fireEvent.keyDown(field(), { key: "Enter" }); + expect(useFindPopup.getState().open).toBe(false); + expect(replaced).toHaveLength(0); + }); + + it("rejects Replace All if the query changes while confirmation is open", async () => { + render(); + act(() => showFindPopup(true, { query: "order" })); + await settle(); + let confirm!: (ok: boolean) => void; + confirmation = new Promise((r) => { + confirm = r; + }); + fireEvent.click(screen.getByRole("button", { name: "Replace All" })); + expect(confirms).toHaveLength(1); + fireEvent.change(field(), { target: { value: "ord" } }); + await settle(); + await act(async () => confirm(true)); + expect(replaced).toHaveLength(0); + }); +}); diff --git a/src/features/search/__tests__/findSession.test.ts b/src/features/search/__tests__/findSession.test.ts index bc8c8f6a2..58b74eb79 100644 --- a/src/features/search/__tests__/findSession.test.ts +++ b/src/features/search/__tests__/findSession.test.ts @@ -96,6 +96,59 @@ describe("FindSession", () => { expect(last().status).toBe("searching"); }); + it("invalidates before debounce, drops old events, and keeps rows until the first new file", async () => { + const { session, deps, running, last } = setup(); + void session.search(q("foo")); + await flush(); + running[0]!.onFile(file("old.kt", [1, 2])); + await flush(); + const oldFiles = last().files; + session.invalidate(); + expect(deps.cancel).toHaveBeenCalledWith(1); + expect(last()).toMatchObject({ stale: true, matchCount: 0, summary: null }); + expect(last().files).toBe(oldFiles); + running[0]!.onFile(file("late.kt", [3])); + running[0]!.finish(summary(3)); + await flush(); + expect(last().files).toBe(oldFiles); + let validate!: () => void; + vi.mocked(deps.validate).mockImplementationOnce( + () => + new Promise((r) => { + validate = r; + }), + ); + void session.search(q("bar")); + expect(last().stale).toBe(true); + expect(last().files).toBe(oldFiles); + validate(); + await flush(); + expect(last().status).toBe("searching"); + expect(last().stale).toBe(true); + expect(last().files).toBe(oldFiles); + running[1]!.onFile(file("new.kt", [4])); + await flush(); + expect(last().stale).toBe(false); + expect(last().files.map((f) => f.relPath)).toEqual(["new.kt"]); + expect(last().matchCount).toBe(1); + }); + + it("clears retained rows on completion without results", async () => { + const { session, running, last } = setup(); + void session.search(q("foo")); + await flush(); + running[0]!.onFile(file("old.kt", [1])); + running[0]!.finish(summary(1)); + await flush(); + void session.search(q("absent")); + await flush(); + expect(last().stale).toBe(true); + expect(last().files).toHaveLength(1); + running[1]!.finish({ ...summary(0), files: 0 }); + await flush(); + expect(last()).toMatchObject({ status: "done", stale: false, files: [], matchCount: 0 }); + }); + it("reports invalid patterns without starting a search", async () => { const { session, deps, last } = setup(); await session.search(q("(", { isRegex: true })); diff --git a/src/features/search/find/FindInFiles.module.css b/src/features/search/find/FindInFiles.module.css index 76c4ed1de..da28768ad 100644 --- a/src/features/search/find/FindInFiles.module.css +++ b/src/features/search/find/FindInFiles.module.css @@ -120,6 +120,12 @@ .results > * { flex: 1; } +.outdated .rowText, +.outdated .rowText *, +.outdated .rowFile, +.outdated .rowFile * { + color: var(--fg-faint); +} .rowText { flex: 1; min-width: 0; diff --git a/src/features/search/find/FindInFiles.tsx b/src/features/search/find/FindInFiles.tsx index 957b8bba1..f265465fb 100644 --- a/src/features/search/find/FindInFiles.tsx +++ b/src/features/search/find/FindInFiles.tsx @@ -193,8 +193,12 @@ function FindInFilesDialog() { ); const queryKey = JSON.stringify(query); + const stale = state.stale || (state.query ? JSON.stringify(state.query) !== queryKey : !!s.pattern); + const currentQuery = useRef(queryKey); const [rerun, setRerun] = useState(0); - useEffect(() => { + useLayoutEffect(() => { + currentQuery.current = queryKey; + session.invalidate(); const t = setTimeout(() => { void session.search(query); setIndex(0); @@ -209,13 +213,13 @@ function FindInFilesDialog() { const selected = active >= 0 ? rows[active] : undefined; const chosen = useMemo(() => selectedRows(rows, sel), [rows, sel]); const fileHighlights = useMemo(() => { - if (!selected) return []; + if (stale || !selected) return []; const f = state.files.find((x) => x.path === selected.path); return (f?.matches ?? []).map((m) => ({ line: m.line, ranges: m.ranges.map(([a, b]) => [a + m.textOffset, b + m.textOffset] as [number, number]), })); - }, [selected, state.files]); + }, [stale, selected, state.files]); const close = () => { rememberFindString("find", s.pattern); @@ -243,8 +247,13 @@ function FindInFilesDialog() { openInFindWindow(pinned, describeQuery(pinned, root), s.replace ? s.replacement : null, undefined, s.preserveCase); }; + // Check again after a confirmation: its captured rows may have been superseded. + const canReplace = () => + !stale && !session.current.stale && currentQuery.current === queryKey && session.current.files === state.files; + /** IDEA asks before Replace All only when the results span more than one file. */ const confirmReplaceAll = async () => { + if (!canReplace()) return; const usages = pendingUsages(rows, marks); if (!usages.length) return; const counts = countUsages(usages); @@ -278,7 +287,7 @@ function FindInFilesDialog() { /** Replaces `usages` in the backend, then marks them done (IDEA's Replace / Replace All). */ const replaceUsages = async (usages: Usage[]): Promise => { - if (!usages.length || running) return null; + if (!canReplace() || !usages.length || running) return null; const run = startReplace( { query, replacement: s.replacement, preserveCase: s.preserveCase, targets: replaceTargets(usages) }, setProgress, @@ -349,10 +358,18 @@ function FindInFilesDialog() { if (done) close(); }; + const diffPreviewKey = JSON.stringify([ + selected?.key, + queryKey, + s.replacement, + s.preserveCase, + rerun, + previewVersion, + ]); useEffect(() => { - if (!previewMode || !s.replace || !selected) return; + if (stale || !previewMode || !s.replace || !selected) return; let alive = true; - const key = selected.key; + const key = diffPreviewKey; const usages = pendingUsages([selected], marks); const t = setTimeout(() => { loadReplacePreview(query, s.replacement, s.preserveCase, selected.path, usages).then( @@ -365,7 +382,7 @@ function FindInFilesDialog() { clearTimeout(t); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [previewMode, s.replace, selected?.key, s.replacement, s.preserveCase, queryKey, marks, previewVersion]); + }, [stale, previewMode, s.replace, diffPreviewKey, marks]); const moveTo = (next: number, extend: boolean) => setSel((cur) => selectRow(rows, cur, next, extend ? "range" : "single")); @@ -443,7 +460,7 @@ function FindInFilesDialog() { return () => setFindPopupCommandHandler(null); }, []); - const summary = statusText(state); + const summary = stale ? (s.pattern ? tx("Updating results…") : "") : statusText(state); const maskItems: MenuItem[] = recentMasks().map((m) => ({ label: m, checked: m === s.mask ? true : undefined, @@ -467,7 +484,7 @@ function FindInFilesDialog() { checked: c.value === s.context ? true : undefined, run: () => set({ context: c.value }), })); - const pendingSelected = s.replace ? pendingUsages(chosen, marks).length : 0; + const pendingSelected = s.replace && !stale ? pendingUsages(chosen, marks).length : 0; return ( {s.replace ? tx("Replace in Files") : tx("Find in Files")} - {state.status === "searching" || state.status === "validating" ? : null} + {stale || state.status === "searching" || state.status === "validating" ? : null} {summary} @@ -664,7 +681,11 @@ function FindInFilesDialog() { )} -
+
items={rows} getKey={(r, i) => `${r.key}:${i}`} @@ -691,10 +712,10 @@ function FindInFilesDialog() { > @@ -725,9 +746,9 @@ function FindInFilesDialog() { )}
- {previewMode && s.replace && selected ? ( + {!stale && previewMode && s.replace && selected ? ( ) : ( @@ -760,19 +781,19 @@ function FindInFilesDialog() { {s.replace && ( <> - - -