mirror of
https://github.com/Hmbown/DeepSeek-TUI.git
synced 2026-09-03 06:50:13 +08:00
fix(web): highlight search matches using the source string's own indices
Defect: both search surfaces carried an identical `highlight()` that
took `idx` from `text.toLowerCase()` and then sliced `text` with it:
const lower = text.toLowerCase();
const idx = lower.indexOf(q);
text.slice(idx, idx + q.length)
That assumes lowercasing preserves length. It does not — `"İ"` (U+0130,
Turkish dotted capital I) lowercases to two code units, so every index
after one is off by one. Searching "stanbul" in "İstanbul kurulumu"
marks "tanbul " instead of "stanbul": the highlight starts and ends one
character late. Turkish is a routed locale (`tr`), and the search
haystack is where localized copy is headed.
Fix: `highlightSpan()` in lib/search-utils.ts — the module that exists
so the search components' pure logic can be unit-tested — folds case
one character at a time and keeps a position map, so the three returned
pieces are cut at real character boundaries and always reassemble the
input exactly. Both components now call it instead of repeating the
arithmetic.
Scope, honestly: no string in the current haystack contains a
length-changing character, so this is a latent bug rather than an
observed one. It is still wrong output for valid input, and it was
wrong in two places.
Evidence: lib/search-utils.test.ts. Restoring the lowercased-index
implementation fails "keeps indices in the source string when
lowercasing changes length" with `expected 'tanbul ' to be 'stanbul'`.
The last case pins both components onto the shared rule.
npm test 327 passed, npm run lint clean, npx tsc --noEmit clean.
Implemented with agent assistance.
Signed-off-by: Hunter Bown <hmbown@gmail.com>
This commit is contained in:
@@ -8,7 +8,7 @@ import {
|
||||
docTopicIsExternal,
|
||||
type DocTopic,
|
||||
} from "@/lib/docs-map";
|
||||
import { docTopicHaystack } from "@/lib/search-utils";
|
||||
import { docTopicHaystack, highlightSpan } from "@/lib/search-utils";
|
||||
|
||||
/* ------------------------------------------------------------------ */
|
||||
/* Locale-aware strings */
|
||||
@@ -43,16 +43,16 @@ const topicHaystack = docTopicHaystack;
|
||||
/* ------------------------------------------------------------------ */
|
||||
|
||||
function highlight(text: string, query: string): React.ReactNode {
|
||||
const q = query.trim().toLowerCase();
|
||||
if (!q) return text;
|
||||
const lower = text.toLowerCase();
|
||||
const idx = lower.indexOf(q);
|
||||
if (idx === -1) return text;
|
||||
// Index arithmetic lives in search-utils: lowercasing can change a
|
||||
// string's length, so `text` cannot be sliced with indices taken from
|
||||
// its lowercased copy.
|
||||
const span = highlightSpan(text, query);
|
||||
if (!span) return text;
|
||||
return (
|
||||
<>
|
||||
{text.slice(0, idx)}
|
||||
<mark className="search-highlight">{text.slice(idx, idx + q.length)}</mark>
|
||||
{text.slice(idx + q.length)}
|
||||
{span.before}
|
||||
<mark className="search-highlight">{span.match}</mark>
|
||||
{span.after}
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { useState, useMemo, useRef, useCallback, useEffect } from "react";
|
||||
import { faqSourceHref } from "@/lib/faq-source";
|
||||
import { extractText } from "@/lib/react-text";
|
||||
import { highlightSpan } from "@/lib/search-utils";
|
||||
|
||||
export interface FaqSearchItem {
|
||||
q: string;
|
||||
@@ -15,16 +16,16 @@ export interface FaqSearchItem {
|
||||
/* ------------------------------------------------------------------ */
|
||||
|
||||
function highlight(text: string, query: string): React.ReactNode {
|
||||
const q = query.trim().toLowerCase();
|
||||
if (!q) return text;
|
||||
const lower = text.toLowerCase();
|
||||
const idx = lower.indexOf(q);
|
||||
if (idx === -1) return text;
|
||||
// Index arithmetic lives in search-utils: lowercasing can change a
|
||||
// string's length, so `text` cannot be sliced with indices taken from
|
||||
// its lowercased copy.
|
||||
const span = highlightSpan(text, query);
|
||||
if (!span) return text;
|
||||
return (
|
||||
<>
|
||||
{text.slice(0, idx)}
|
||||
<mark className="search-highlight">{text.slice(idx, idx + q.length)}</mark>
|
||||
{text.slice(idx + q.length)}
|
||||
{span.before}
|
||||
<mark className="search-highlight">{span.match}</mark>
|
||||
{span.after}
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import {
|
||||
docTopicHaystack,
|
||||
filterDocTopics,
|
||||
highlightSpan,
|
||||
normalizeQuery,
|
||||
matches,
|
||||
} from "./search-utils";
|
||||
@@ -129,3 +131,49 @@ describe("filterDocTopics", () => {
|
||||
expect(ids).toContain("install");
|
||||
});
|
||||
});
|
||||
|
||||
describe("highlightSpan", () => {
|
||||
const webRoot = new URL("../", import.meta.url);
|
||||
const read = (p: string) => readFileSync(new URL(p, webRoot), "utf8");
|
||||
|
||||
it("splits a plain match into three reassembling pieces", () => {
|
||||
const span = highlightSpan("Install Guide", "install")!;
|
||||
expect(span).toEqual({ before: "", match: "Install", after: " Guide" });
|
||||
expect(span.before + span.match + span.after).toBe("Install Guide");
|
||||
});
|
||||
|
||||
it("returns null for an empty query or no match", () => {
|
||||
expect(highlightSpan("Install", "")).toBeNull();
|
||||
expect(highlightSpan("Install", " ")).toBeNull();
|
||||
expect(highlightSpan("Install", "docker")).toBeNull();
|
||||
expect(highlightSpan("", "install")).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps indices in the source string when lowercasing changes length", () => {
|
||||
// "İ".toLowerCase() is two code units, so a lowercased copy of this
|
||||
// string is one longer than the string itself. Index arithmetic done on
|
||||
// the copy highlighted "tanbul " instead of "stanbul".
|
||||
const text = "İstanbul kurulumu";
|
||||
expect(text.toLowerCase().length).toBe(text.length + 1);
|
||||
const span = highlightSpan(text, "stanbul")!;
|
||||
expect(span.match).toBe("stanbul");
|
||||
expect(span.before).toBe("İ");
|
||||
expect(span.after).toBe(" kurulumu");
|
||||
expect(span.before + span.match + span.after).toBe(text);
|
||||
});
|
||||
|
||||
it("never claims half of a source character", () => {
|
||||
const span = highlightSpan("İstanbul", "i")!;
|
||||
expect(span.match).toBe("İ");
|
||||
expect(span.before + span.match + span.after).toBe("İstanbul");
|
||||
});
|
||||
|
||||
it("is the one match rule both search surfaces use", () => {
|
||||
for (const file of ["components/docs-search.tsx", "components/faq-search.tsx"]) {
|
||||
const source = read(file);
|
||||
expect(source, file).toContain("highlightSpan(text, query)");
|
||||
expect(source, file).not.toContain("text.toLowerCase()");
|
||||
expect(source, file).not.toContain("text.slice(");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -64,3 +64,60 @@ export function matches(haystack: string, query: string): boolean {
|
||||
if (!q) return true;
|
||||
return haystack.toLowerCase().includes(q);
|
||||
}
|
||||
|
||||
/** The three pieces a highlighted match splits a string into. */
|
||||
export interface HighlightSpan {
|
||||
before: string;
|
||||
match: string;
|
||||
after: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Locate `query` inside `text`, case-insensitively, in `text`'s own indices.
|
||||
*
|
||||
* The obvious form — `text.toLowerCase().indexOf(q)`, then slicing `text`
|
||||
* with that index — assumes lowercasing preserves length. It does not:
|
||||
* `"İ".toLowerCase()` is two code units, so every index after a dotted
|
||||
* capital I in the haystack is off by one and the highlight lands on the
|
||||
* wrong characters. Turkish is a routed locale, so this is reachable the
|
||||
* moment localized copy enters the search haystack.
|
||||
*
|
||||
* Lowercasing character by character and keeping a position map costs one
|
||||
* pass and keeps the three returned pieces exactly reassembling `text`.
|
||||
* Returns null when there is no match (including an empty query).
|
||||
*/
|
||||
export function highlightSpan(text: string, query: string): HighlightSpan | null {
|
||||
const q = normalizeQuery(query);
|
||||
if (!q) return null;
|
||||
|
||||
let lower = "";
|
||||
// For each code unit of `lower`: where its source character starts and ends.
|
||||
const sourceStart: number[] = [];
|
||||
const sourceEnd: number[] = [];
|
||||
for (let i = 0; i < text.length; ) {
|
||||
const char = String.fromCodePoint(text.codePointAt(i)!);
|
||||
const next = i + char.length;
|
||||
const folded = char.toLowerCase();
|
||||
for (let k = 0; k < folded.length; k++) {
|
||||
sourceStart.push(i);
|
||||
sourceEnd.push(next);
|
||||
}
|
||||
lower += folded;
|
||||
i = next;
|
||||
}
|
||||
|
||||
const idx = lower.indexOf(q);
|
||||
if (idx === -1) return null;
|
||||
|
||||
const start = sourceStart[idx];
|
||||
const stop = idx + q.length;
|
||||
// A match ending inside one source character's expansion cannot claim half
|
||||
// of that character; take the whole character rather than nothing.
|
||||
const end = stop < lower.length ? Math.max(sourceStart[stop], sourceEnd[idx]) : text.length;
|
||||
|
||||
return {
|
||||
before: text.slice(0, start),
|
||||
match: text.slice(start, end),
|
||||
after: text.slice(end),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user