fix(search): mark outdated find results and block stale replacements (#630)

While a new query was debouncing or validating, Find and Replace in Files kept showing the previous query's rows, count and highlights, and Replace stayed enabled. Replace, Replace All in This File and Replace All then sent the new pattern with the old positions, so `order` changed to `ord` could rewrite `order` as `Xer`.

From the moment query, flags, mask, scope or context change, old rows are dimmed and lose their highlights, replacement preview and count until the new search's first results arrive. All replace buttons and keyboard paths are blocked meanwhile, and a pending Replace All confirmation rechecks that its results are still current. Opening a retained row still works.

Refs #623
Цей коміт міститься в:
l0ng-ai authored and GitHub committed 2026-11-01 09:55:43 +08:00
1 джерело 1e913c2334
коміт b281447da8
6 files changed
+308 -30

No files matched your search

+176 -4
Переглянути файл
@@ -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<void> | null = null;
let search: ((onFile: (file: FileMatches) => void) => Promise<SearchSummary | null>) | null = null;
let confirmation: Promise<boolean> | null = null;
vi.mock("../find/windowStore", async (orig) => ({
...(await orig<typeof import("../find/windowStore")>()),
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<typeof import("@/workbench/messages")>()),
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(<FindInFilesOverlay />);
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<void>((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(<FindInFilesOverlay />);
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(<FindInFilesOverlay />);
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(<FindInFilesOverlay />);
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(<FindInFilesOverlay />);
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(<FindInFilesOverlay />);
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);
});
});
+53
Переглянути файл
@@ -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<void>((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 }));
+6
Переглянути файл
@@ -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;
+40 -19
Переглянути файл
@@ -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<ReplaceMarks | null> => {
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 (
<Dialog
@@ -486,7 +503,7 @@ function FindInFilesDialog() {
<header className={styles.header} data-dialog-drag="">
<span className={styles.title}>{s.replace ? tx("Replace in Files") : tx("Find in Files")}</span>
<span className={styles.status} aria-live="polite">
{state.status === "searching" || state.status === "validating" ? <Spinner size={12} /> : null}
{stale || state.status === "searching" || state.status === "validating" ? <Spinner size={12} /> : null}
{summary}
</span>
<span className={styles.spacer} />
@@ -664,7 +681,11 @@ function FindInFilesDialog() {
)}
</div>
</div>
<div className={styles.results}>
<div
className={clsx(styles.results, stale && styles.outdated)}
data-stale={stale || undefined}
aria-busy={stale || state.status === "searching"}
>
<List<MatchRow>
items={rows}
getKey={(r, i) => `${r.key}:${i}`}
@@ -691,10 +712,10 @@ function FindInFilesDialog() {
>
<MatchText
text={r.text}
ranges={r.ranges}
ranges={stale ? [] : r.ranges}
source={{ path: r.path, line: r.line, col: r.textCol }}
active={st.active}
replacement={s.replace && state !== "done" ? s.replacement : null}
replacement={!stale && s.replace && state !== "done" ? s.replacement : null}
query={query}
preserveCase={s.preserveCase}
/>
@@ -725,9 +746,9 @@ function FindInFilesDialog() {
)}
</div>
<div className={styles.previewBox}>
{previewMode && s.replace && selected ? (
{!stale && previewMode && s.replace && selected ? (
<ReplaceDiffPreview
lines={diffPreview?.key === selected.key ? diffPreview.lines : null}
lines={diffPreview?.key === diffPreviewKey ? diffPreview.lines : null}
line={selected.line}
/>
) : (
@@ -760,19 +781,19 @@ function FindInFilesDialog() {
{s.replace && (
<>
<Checkbox checked={previewMode} onChange={setPreviewMode} label="Preview" />
<Button size="small" variant="secondary" disabled={!selected || busy} onClick={skip}>
<Button size="small" variant="secondary" disabled={stale || !selected || busy} onClick={skip}>
<T>Skip</T>
</Button>
<Button size="small" disabled={!selected || busy} onClick={() => void replaceSelected()}>
<Button size="small" disabled={stale || !selected || busy} onClick={() => void replaceSelected()}>
{pendingSelected > 1 ? txt(`Replace ${pendingSelected} Occurrences`) : <T>Replace</T>}
</Button>
<Button size="small" disabled={!selected || busy} onClick={() => void replaceInFile()}>
<Button size="small" disabled={stale || !selected || busy} onClick={() => void replaceInFile()}>
<T>Replace All in This File</T>
</Button>
<Button
size="small"
title="Replace All (⇧⌥↵, ⌥A)"
disabled={!rows.length || busy || state.status === "searching"}
disabled={stale || !rows.length || busy || state.status === "searching"}
onClick={() => void confirmReplaceAll()}
>
<T>Replace All</T>
+32 -7
Переглянути файл
@@ -17,6 +17,8 @@ export type FindStatus = "idle" | "validating" | "invalid" | "searching" | "done
export interface FindState {
status: FindStatus;
query: SearchQuery | null;
/** Retained rows belong to a superseded search. */
stale: boolean;
files: readonly FileMatches[];
matchCount: number;
summary: SearchSummary | null;
@@ -34,7 +36,15 @@ export interface FindDeps {
export const countFileMatches = (file: FileMatches): number =>
file.matches.reduce((count, line) => count + line.ranges.length, 0);
export const IDLE: FindState = { status: "idle", query: null, files: [], matchCount: 0, summary: null, error: null };
export const IDLE: FindState = {
status: "idle",
query: null,
stale: false,
files: [],
matchCount: 0,
summary: null,
error: null,
};
function errorMessage(e: unknown): string {
if (e && typeof e === "object" && "message" in e) return String((e as { message: unknown }).message);
@@ -61,6 +71,13 @@ export class FindSession {
return this.state;
}
/** Invalidates immediately, before a debounced search starts, keeping the layout steady. */
invalidate() {
this.seq++;
this.cancelRunning();
this.update({ ...this.state, stale: true, matchCount: 0, summary: null, error: null }, true);
}
/** Starts (or restarts) a search; an empty pattern resets to idle. */
async search(query: SearchQuery): Promise<void> {
const seq = ++this.seq;
@@ -70,7 +87,10 @@ export class FindSession {
this.update({ ...IDLE }, true);
return;
}
this.update({ ...this.state, status: "validating", query, error: null }, true);
this.update(
{ ...this.state, status: "validating", query, stale: true, matchCount: 0, summary: null, error: null },
true,
);
try {
await this.deps.validate(query);
} catch (e) {
@@ -84,22 +104,27 @@ export class FindSession {
this.request = request;
const files: FileMatches[] = [];
let matchCount = 0;
this.update({ status: "searching", query, files: [], matchCount: 0, summary: null, error: null }, true);
this.update({ ...this.state, status: "searching" }, true);
try {
const summary = await this.deps.start(request, query, (f) => {
if (seq !== this.seq) return;
files.push(f);
matchCount += countFileMatches(f);
this.update({ ...this.state, files: [...files], matchCount });
this.update({ ...this.state, stale: false, files: [...files], matchCount });
});
if (seq !== this.seq) return;
this.request = null;
this.update({ ...this.state, status: "done", files: [...files], matchCount, summary }, true);
this.update({ ...this.state, status: "done", stale: false, files: [...files], matchCount, summary }, true);
} catch (e) {
if (seq !== this.seq) return;
this.request = null;
if (isCancelledError(e)) this.update({ ...this.state, status: "done", files: [...files], matchCount }, true);
else this.update({ ...this.state, status: "error", error: errorMessage(e) }, true);
if (isCancelledError(e))
this.update({ ...this.state, status: "done", stale: false, files: [...files], matchCount }, true);
else
this.update(
{ ...this.state, status: "error", stale: false, files: [...files], matchCount, error: errorMessage(e) },
true,
);
}
}
+1
Переглянути файл
@@ -1,4 +1,5 @@
{
"Updating results…": "正在更新结果…",
"Uploading files": "正在上传文件",
"No files on the clipboard": "剪贴板中没有文件",
"File name is not valid UTF-8": "文件名不是有效的 UTF-8",