diff --git a/README.md b/README.md index 08993d0..eccd34c 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ Completion runs in the shell alone and never starts diffle, so it stays fast — - `V`: select a block - `v`: mark viewed - `/`: search the current file -- `g/`: search the diff or codebase +- `g/`: search changed files - `gf`: filter files - `yy`: copy all comments - `F`: open the full file @@ -122,6 +122,8 @@ Completion runs in the shell alone and never starts diffle, so it stays fast — Press `?` in the app for the full list. +Both content searches toggle between diff hunks with context and full file contents, remembering that choice independently. Global search stays within changed files. + ## GitHub reviews Use the pull request icon to add one thread or all open threads to a pending GitHub review — a new one, or the pending review already waiting on the pull request. This requires a local, authenticated [`gh`](https://cli.github.com/). diff --git a/src/client/api.ts b/src/client/api.ts index 27a7454..a8dfc98 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -34,6 +34,7 @@ import type { ModeRequest, ReplyCreate, SearchScope, + SearchContent, ServerMessage, Side, ThreadCreate, @@ -111,11 +112,18 @@ export const api = { /** `path` names the file a `scope: 'file'` search is confined to. */ search: ( query: string, - opts: { word?: boolean; ignoreCase?: boolean; regex?: boolean; scope?: SearchScope; path?: string } = {}, + opts: { + word?: boolean; + ignoreCase?: boolean; + regex?: boolean; + scope?: SearchScope; + content?: SearchContent; + path?: string; + } = {}, ) => json( SearchResponseSchema, - `api/search?${q({ q: query, word: opts.word ? '1' : undefined, i: opts.ignoreCase ? '1' : undefined, re: opts.regex ? '1' : undefined, scope: opts.scope, path: opts.path })}`, + `api/search?${q({ q: query, word: opts.word ? '1' : undefined, i: opts.ignoreCase ? '1' : undefined, re: opts.regex ? '1' : undefined, scope: opts.scope, content: opts.content, path: opts.path })}`, ), threads: (query: ThreadQuery = {}) => json(CommentThreadSchema.array(), `api/threads?${q({ state: query.state, path: query.path })}`), diff --git a/src/client/keyboard/HelpOverlay.tsx b/src/client/keyboard/HelpOverlay.tsx index 212b17e..b445b48 100644 --- a/src/client/keyboard/HelpOverlay.tsx +++ b/src/client/keyboard/HelpOverlay.tsx @@ -53,7 +53,7 @@ export const COLUMNS: HelpSection[][] = [ title: 'Search', rows: [ ['/', 'search the current file, then n / N between matches'], - ['g/', 'search the diff (scope button: file / diff / codebase)'], + ['g/', 'search changed files (toggle: diff + context / full file)'], ['⌘/Ctrl+p or gf', 'search files by name'], ['w / b', 'focus the next / previous symbol on the line'], ['0 / $', 'focus the first / last symbol on the line'], diff --git a/src/client/keyboard/SearchBar.tsx b/src/client/keyboard/SearchBar.tsx index dc56431..e141524 100644 --- a/src/client/keyboard/SearchBar.tsx +++ b/src/client/keyboard/SearchBar.tsx @@ -1,19 +1,10 @@ import { twMerge } from 'tailwind-merge'; import { Button } from '../ui/Button.js'; -import { FileDiff, FileSearch, FolderSearch, Link2, Search, WholeWord, X } from 'lucide-react'; +import { FileDiff, FileSearch, Link2, Search, WholeWord, X } from 'lucide-react'; import { useEffect, useRef } from 'react'; -import type { SearchScope } from '../../shared/protocol.js'; -import { nextSearchScope } from '../model.js'; import { useStore } from '../store.js'; -const SCOPE_LABEL: Record = { - file: 'the current file', - diff: 'only the diff', - repo: 'the full codebase', -}; -const SCOPE_ICON: Record = { file: FileSearch, diff: FileDiff, repo: FolderSearch }; - -/** Content search (/ in the current file, g/ across the diff or codebase), the references of a symbol (gA), or a word's occurrences (* / #). Enter runs a search; n / N step through matches. */ +/** Content search (/ in the current file, g/ across changed files), the references of a symbol (gA), or a word's occurrences (* / #). Enter runs a search; n / N step through matches. */ export function SearchBar({ path }: { path?: string }) { const visible = useStore((s) => { const local = s.search.kind === 'text' && s.search.scope === 'file'; @@ -57,7 +48,8 @@ function SearchForm() { ); } - const ScopeIcon = SCOPE_ICON[search.scope]; + const fullFile = search.content[search.scope] === 'full'; + const ScopeIcon = fullFile ? FileSearch : FileDiff; return (
setSearchOptions({ scope: nextSearchScope(search.scope) })} - title={`Searching ${SCOPE_LABEL[search.scope]}; click to search ${SCOPE_LABEL[nextSearchScope(search.scope)]}`} + aria-label="Search full file" + aria-pressed={fullFile} + onClick={() => setSearchOptions({ content: fullFile ? 'diff' : 'full' })} + title={ + fullFile + ? 'Searching full file; click to search diff + context' + : 'Searching diff + context; click to search full file' + } > @@ -128,9 +124,9 @@ function SearchForm() { : n === 0 && search.query ? search.scope === 'file' ? 'none in file' - : search.scope === 'diff' - ? 'none in diff' - : 'no matches' + : fullFile + ? 'no matches' + : 'none in diff' : n ? `${search.index + 1} / ${n}${search.truncated ? '+' : ''}` : ''} diff --git a/src/client/keyboard/useKeymap.ts b/src/client/keyboard/useKeymap.ts index a3dc244..ad09fd8 100644 --- a/src/client/keyboard/useKeymap.ts +++ b/src/client/keyboard/useKeymap.ts @@ -2,7 +2,7 @@ import { useEffect, useRef } from 'react'; import { copyText } from '../clipboard.js'; import { api } from '../api.js'; import { clearWordFocus, moveWord, moveWordToEdge } from '../lsp/wordNav.js'; -import { currentPath, widenSearchScope } from '../model.js'; +import { currentPath } from '../model.js'; import { remPx } from '../scale.js'; import { useStore, type ReviewState } from '../store.js'; import { nextTheme } from '../theme.js'; @@ -63,7 +63,7 @@ const KEYMAP: Record = { yy: () => void copyComments(), Y: () => void copyComments(), '/': (s) => s.openSearch('file'), - 'g/': (s) => s.openSearch(widenSearchScope(s.search.scope)), + 'g/': (s) => s.openSearch('diff'), gf: () => focusFileSearch(), gd: (s) => s.goToDefinition(), gy: (s) => s.goToTypeDefinition(), diff --git a/src/client/model.ts b/src/client/model.ts index 3c829fe..2c06e7a 100644 --- a/src/client/model.ts +++ b/src/client/model.ts @@ -6,7 +6,6 @@ import type { CommentThread, LspSymbol, ModeRequest, - SearchScope, Snapshot, UserConfig, ViewedState, @@ -110,18 +109,6 @@ export function currentPath(state: Pick { - return scope === 'file' ? 'diff' : scope; -} - /** * The order `@pierre/trees` lists paths in (its path store's default sort), so the review pane and * the tree agree file for file: at the first segment where two paths diverge, folders before diff --git a/src/client/store.ts b/src/client/store.ts index 9c319a9..d609995 100644 --- a/src/client/store.ts +++ b/src/client/store.ts @@ -17,6 +17,7 @@ import { type ModeRequest, type SearchMatch, type SearchScope, + type SearchContent, type Side, type Snapshot, type UserConfig, @@ -94,8 +95,10 @@ export interface SearchState { /** Text-search options; `*` / `#` ignore them (whole word, case-sensitive, whole repository). */ ignoreCase: boolean; regex: boolean; - /** 'file' searches the file the reader is in; 'diff' only the changed files; 'repo' the whole codebase. */ - scope: SearchScope; + /** Local search stays pinned to one file; global search covers changed files. */ + scope: Exclude; + /** Each search location remembers its own content mode. */ + content: Record, SearchContent>; /** The file owning the local search bar and its results, pinned when search opens. */ path: string | null; /** Bumped by `g/` so the box takes focus again after `n` / `N` blurred it. */ @@ -236,15 +239,15 @@ export interface ReviewState { /** Toast for a failed fire-and-forget action: `${what} failed: `. */ report(what: string, e: unknown): void; search: SearchState; - /** Open the text search box; `scope` replaces the remembered scope (`/` → file, `g/` → widened). */ - openSearch(scope?: SearchScope): void; + /** Open the text search box; `scope` replaces the remembered scope (`/` → file, `g/` → diff). */ + openSearch(scope?: SearchState['scope']): void; setSearchInput(value: string): void; blurSearchInput(): void; typeSearchInput(key: string): void; closeSearch(): void; runSearch(query: string): Promise; /** Flip a text-search option and rerun the current query. */ - setSearchOptions(opts: Partial>): void; + setSearchOptions(opts: Partial> & { content?: SearchContent }): void; moveMatch(delta: 1 | -1): void; /** `*` / `#`: whole-word search for the focused word, landing on the next occurrence in `delta`'s direction. */ searchWord(delta: 1 | -1): Promise; @@ -1304,6 +1307,7 @@ export const useStore = create((set, get) => { ignoreCase: true, regex: false, scope: 'diff', + content: { file: 'diff', diff: 'diff' }, path: null, focusNonce: 0, input: '', @@ -1315,9 +1319,11 @@ export const useStore = create((set, get) => { truncated: false, }, openSearch(scope) { + searchSeq.start(); set((s) => { const nextScope = scope ?? s.search.scope; const path = nextScope === 'file' ? currentPath(s) : null; + const sameSearch = s.search.kind === 'text' && s.search.scope === nextScope && s.search.path === path; return { search: { ...s.search, @@ -1325,6 +1331,10 @@ export const useStore = create((set, get) => { kind: 'text', direction: 1, scope: nextScope, + matches: sameSearch ? s.search.matches : [], + index: sameSearch ? s.search.index : -1, + truncated: sameSearch && s.search.truncated, + loading: false, input: s.search.open && s.search.kind === 'text' ? s.search.input : s.search.query, editing: true, path, @@ -1370,11 +1380,12 @@ export const useStore = create((set, get) => { })); }, setSearchOptions(opts) { + const { content, ...options } = opts; set((s) => ({ search: { ...s.search, - ...opts, - path: opts.scope === undefined ? s.search.path : opts.scope === 'file' ? currentPath(s) : null, + ...options, + content: content ? { ...s.search.content, [s.search.scope]: content } : s.search.content, }, })); const { query, kind } = get().search; @@ -1405,12 +1416,18 @@ export const useStore = create((set, get) => { search: { ...s.search, kind: 'text', direction: 1, query, input: query, editing: false, path, loading: true }, })); try { - const { ignoreCase, regex, scope } = get().search; + const { ignoreCase, regex, scope, content } = get().search; if (scope === 'file' && !path) { set((s) => ({ search: { ...s.search, matches: [], index: -1, loading: false } })); return get().flash('No file to search in'); } - const res = await api.search(query, { ignoreCase, regex, scope, ...(path ? { path } : {}) }); + const res = await api.search(query, { + ignoreCase, + regex, + scope, + content: content[scope], + ...(path ? { path } : {}), + }); if (!searchOwned(g, t)) return; set((s) => ({ search: { ...s.search, matches: res.matches, truncated: res.truncated, index: -1, loading: false }, @@ -1420,9 +1437,7 @@ export const useStore = create((set, get) => { get().flash( scope === 'file' ? `No matches for “${query}” in ${path}` - : scope === 'diff' - ? `No matches for “${query}” in the diff` - : `No matches for “${query}”`, + : `No matches for “${query}” in ${content[scope] === 'diff' ? 'the diff + context' : 'changed files'}`, ); } catch (e) { if (!searchOwned(g, t)) return; @@ -1459,7 +1474,7 @@ export const useStore = create((set, get) => { }, })); try { - const res = await api.search(word, { word: true, scope: 'repo' }); + const res = await api.search(word, { word: true, scope: 'repo', content: 'full' }); if (!searchOwned(g, t)) return; const matches = res.matches; // Start from the occurrence just past the cursor in the requested direction, wrapping like vim. diff --git a/src/server/git/GitRepo.ts b/src/server/git/GitRepo.ts index 675a688..60ab83a 100644 --- a/src/server/git/GitRepo.ts +++ b/src/server/git/GitRepo.ts @@ -69,6 +69,7 @@ interface ExecOptions { interface RecordOptions extends ExecOptions { /** Kill the child and reject after this long. */ timeoutMs?: number; + accept?: (record: string) => boolean; } /** The only module that spawns git. All calls run with cwd = repo root, except reading a submodule's HEAD inside it. */ @@ -395,18 +396,27 @@ export class GitRepo { * Worktree searches include untracked files. The result is * bounded globally: git is stopped once `limit + 1` records have arrived, so a * common query in a large repository cannot flood the process. `paths` - * restricts the search to those files; an empty list matches nothing. + * restricts files; `ranges` restricts new-side lines before applying the limit. + * An empty path list or range map matches nothing. */ async grep( query: string, rev: string | 'worktree', limit = 500, - opts: { word?: boolean; ignoreCase?: boolean; regex?: boolean; paths?: string[] } = {}, + opts: { + word?: boolean; + ignoreCase?: boolean; + regex?: boolean; + paths?: string[]; + ranges?: ReadonlyMap; + } = {}, ): Promise<{ matches: { path: string; line: number; text: string }[]; truncated: boolean }> { // An explicit empty path list means "search nothing": with no pathspec git would search everything. - if (!query || opts.paths?.length === 0) return { matches: [], truncated: false }; + if (!query || opts.paths?.length === 0 || opts.ranges?.size === 0) return { matches: [], truncated: false }; // --no-column: `grep.column=true` would splice a column field into the -z record. - const args = ['grep', '-n', '--no-column', '-I', opts.regex ? '-E' : '-F', '-z', `--max-count=${limit + 1}`]; + const args = ['grep', '-n', '--no-column', '-I', opts.regex ? '-E' : '-F', '-z']; + // Filtering must precede the limit: early matches outside hunks must not hide later visible hits. + if (!opts.ranges) args.push(`--max-count=${limit + 1}`); if (opts.word) args.push('-w'); if (opts.ignoreCase) args.push('-i'); args.push('-e', query); @@ -416,13 +426,23 @@ export class GitRepo { // Literal pathspecs: a path with `*` or `?` in it must not turn into a glob. for (const p of opts.paths ?? []) args.push(`:(literal)${p}`); // -z: "path\0line\0text\n" per match; with a rev the path is "rev:path". - const records = await execGitRecords(this.root, args, limit + 1, { okCodes: [0, 1], timeoutMs: GREP_TIMEOUT_MS }); - const truncated = records.length > limit; - const matches = records.slice(0, limit).map((rec) => { + const parse = (rec: string) => { const [rawPath = '', line = '', ...rest] = rec.split('\0'); const path = rev === 'worktree' ? rawPath : rawPath.slice(rawPath.indexOf(':') + 1); return { path, line: Number(line), text: rest.join('\0').slice(0, 300) }; + }; + const records = await execGitRecords(this.root, args, limit + 1, { + okCodes: [0, 1], + timeoutMs: GREP_TIMEOUT_MS, + accept: opts.ranges + ? (record) => { + const { path, line } = parse(record); + return opts.ranges!.get(path)?.some(([start, end]) => line >= start && line <= end) ?? false; + } + : undefined, }); + const truncated = records.length > limit; + const matches = records.slice(0, limit).map(parse); return { matches, truncated }; } @@ -840,9 +860,10 @@ function execGit(cwd: string, args: string[], opts: ExecOptions = {}): Promise { return new Promise((res, rej) => { const child = spawn('git', [...CONFIG_ARGS, ...args], { cwd, env: { ...process.env, GIT_OPTIONAL_LOCKS: '0' } }); - const chunks: Buffer[] = []; + const records: string[] = []; + let chunks: Buffer[] = []; + let fields = 0; const errChunks: Buffer[] = []; - let seen = 0; let done = false; let timedOut = false; const timer = @@ -854,16 +875,25 @@ function execGitRecords(cwd: string, args: string[], maxRecords: number, opts: R }, opts.timeoutMs); child.stdout.on('data', (chunk: Buffer) => { if (done) return; + let start = 0; for (let i = 0; i < chunk.length; i++) { - if (chunk[i] !== 10) continue; - if (++seen === maxRecords) { - chunks.push(chunk.subarray(0, i + 1)); + if (chunk[i] === 0) fields++; + // Git grep terminates path and line with NUL; filenames may themselves contain newlines. + if (chunk[i] !== 10 || fields < 2) continue; + chunks.push(chunk.subarray(start, i)); + const record = Buffer.concat(chunks).toString('utf8'); + chunks = []; + fields = 0; + start = i + 1; + if (opts.accept && !opts.accept(record)) continue; + records.push(record); + if (records.length === maxRecords) { done = true; child.kill(); return; } } - chunks.push(chunk); + if (start < chunk.length) chunks.push(chunk.subarray(start)); }); child.stderr.on('data', (chunk: Buffer) => errChunks.push(chunk)); child.on('error', (err) => rej(new GitError(`git ${args.join(' ')} failed: ${err.message}`, args, null, ''))); @@ -878,8 +908,6 @@ function execGitRecords(cwd: string, args: string[], maxRecords: number, opts: R rej(new GitError(`git ${args.join(' ')} failed: ${stderr.trim()}`, args, code, stderr)); return; } - const records = Buffer.concat(chunks).toString('utf8').split('\n'); - if (records[records.length - 1] === '') records.pop(); res(records); }); }); diff --git a/src/server/routes.ts b/src/server/routes.ts index 686d04e..89eb7c0 100644 --- a/src/server/routes.ts +++ b/src/server/routes.ts @@ -28,6 +28,8 @@ import { lspBlocker, } from '../shared/protocol.js'; import { NotFoundError, UnquotableError } from './comments/CommentStore.js'; +import { shownRanges } from './comments/hunks.js'; +import { mapLimit } from './concurrency.js'; import { formatPrompt } from './comments/format.js'; import { ImportError, parseImports } from './comments/import.js'; import { GitError, isBinary } from './git/GitRepo.js'; @@ -129,19 +131,31 @@ export function createApi(deps: ApiDeps): Hono { const query = SearchQuerySchema.parse(c.req.query()); const { q, scope } = query; const snap = await session.snapshotter.current(); - // Default scope is the diff's new side; `scope=repo` widens to the whole tree, `scope=file` - // narrows to `path`, which must be on the new side (an unknown path matches nothing). - const paths = + // File scope is pinned to the new-side allowlist; global text search covers changed files. + let paths = scope === 'repo' ? undefined : scope === 'file' ? snap.tree.filter((p) => p === query.path) : snap.changed.filter((f) => f.status !== 'D').map((f) => f.path); + let ranges: Map | undefined; + if (query.content === 'diff') { + const candidates = snap.changed.filter( + (f) => f.status !== 'D' && !f.binary && (!paths || paths.includes(f.path)), + ); + const entries = await mapLimit(candidates, 8, async (file) => { + const patch = await session.repo.patch(snap.oldSha, snap.newSha, file, snap.context); + return [file.path, shownRanges(patch, 'new')] as const; + }); + ranges = new Map(entries); + paths = entries.filter(([, spans]) => spans.length > 0).map(([path]) => path); + } const { matches, truncated } = await session.repo.grep(q, snap.newSha, 500, { word: query.word, ignoreCase: query.i, regex: query.re, paths, + ranges, }); const body: SearchResponse = { query: q, matches, truncated }; return c.json(body); diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index 57be388..a8249e6 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -313,6 +313,10 @@ export type SearchMatch = z.infer; export const SearchScopeSchema = z.enum(['file', 'diff', 'repo']); export type SearchScope = z.infer; +/** Whether search includes whole files or only new-side diff hunks and their context. */ +export const SearchContentSchema = z.enum(['diff', 'full']); +export type SearchContent = z.infer; + export const SearchResponseSchema = z.object({ query: z.string(), matches: SearchMatchSchema.array(), @@ -591,6 +595,7 @@ const flagSchema = z export const SearchQuerySchema = z.object({ q: z.string().default(''), scope: SearchScopeSchema.default('diff'), + content: SearchContentSchema.default('diff'), path: z.string().optional(), word: flagSchema, i: flagSchema, diff --git a/test/client/SearchBar.test.ts b/test/client/SearchBar.test.ts new file mode 100644 index 0000000..64abaa7 --- /dev/null +++ b/test/client/SearchBar.test.ts @@ -0,0 +1,66 @@ +// @vitest-environment jsdom +import { act, createElement } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; + +vi.mock('../../src/client/api.js', () => ({ api: {} })); +const { useStore } = await import('../../src/client/store.js'); +const { SearchBar } = await import('../../src/client/keyboard/SearchBar.js'); + +let host: HTMLDivElement; +let root: Root; +beforeEach(() => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + useStore.setState((s) => ({ + activePath: 'a.txt', + search: { + ...s.search, + open: false, + query: '', + input: '', + matches: [], + kind: 'text', + scope: 'diff', + content: { file: 'diff', diff: 'diff' }, + }, + })); + host = document.createElement('div'); + document.body.appendChild(host); + root = createRoot(host); + act(() => + root.render( + createElement( + 'div', + null, + createElement('header', null, createElement(SearchBar)), + createElement('section', null, createElement(SearchBar, { path: 'a.txt' })), + ), + ), + ); +}); +afterEach(() => { + act(() => root.unmount()); + host.remove(); +}); + +it('keeps the local form mounted when toggling and remembers the global mode separately', () => { + act(() => useStore.getState().openSearch('file')); + const input = host.querySelector('section input'); + const toggle = () => host.querySelector('[aria-label="Search full file"]')!; + act(() => toggle().click()); + expect(host.querySelector('section input')).toBe(input); + expect(host.querySelector('header input')).toBeNull(); + expect(toggle().getAttribute('aria-pressed')).toBe('true'); + expect(useStore.getState().search.path).toBe('a.txt'); + + act(() => useStore.getState().openSearch('diff')); + expect(host.querySelector('header input')).not.toBeNull(); + expect(toggle().getAttribute('aria-pressed')).toBe('false'); + act(() => toggle().click()); + expect(host.querySelector('header input')).not.toBeNull(); + act(() => useStore.getState().openSearch('file')); + expect(toggle().getAttribute('aria-pressed')).toBe('true'); + act(() => toggle().click()); + act(() => useStore.getState().openSearch('diff')); + expect(toggle().getAttribute('aria-pressed')).toBe('true'); +}); diff --git a/test/client/highlight.test.ts b/test/client/highlight.test.ts index 2bb31ce..fa774f1 100644 --- a/test/client/highlight.test.ts +++ b/test/client/highlight.test.ts @@ -10,6 +10,7 @@ const base: SearchState = { ignoreCase: true, regex: false, scope: 'diff', + content: { file: 'diff', diff: 'diff' }, path: null, focusNonce: 0, input: 'a.b', diff --git a/test/client/model.test.ts b/test/client/model.test.ts index 5215359..452f594 100644 --- a/test/client/model.test.ts +++ b/test/client/model.test.ts @@ -16,10 +16,8 @@ import { draftRange, isViewed, nextFileAfter, - nextSearchScope, orderedPaths, reuseThreads, - widenSearchScope, } from '../../src/client/model.js'; describe('orderedPaths', () => { @@ -173,18 +171,6 @@ describe('reuseThreads', () => { }); describe('search scope', () => { - it('cycles file → diff → repo → file', () => { - expect(nextSearchScope('file')).toBe('diff'); - expect(nextSearchScope('diff')).toBe('repo'); - expect(nextSearchScope('repo')).toBe('file'); - }); - - it('g/ widens a file search to the diff and keeps a repo choice', () => { - expect(widenSearchScope('file')).toBe('diff'); - expect(widenSearchScope('diff')).toBe('diff'); - expect(widenSearchScope('repo')).toBe('repo'); - }); - it('the current file is the cursor’s, else the whole-file view’s, else the first in tree order', () => { const changed = ['src/b.ts', 'src/a.ts'].map((path) => ({ path, diff --git a/test/client/store.test.ts b/test/client/store.test.ts index f4ede73..5a83bfe 100644 --- a/test/client/store.test.ts +++ b/test/client/store.test.ts @@ -1484,7 +1484,7 @@ describe('symbol navigation', () => { await useStore.getState().searchWord(1); await new Promise((r) => setTimeout(r, 0)); - expect(api.search).toHaveBeenCalledWith('foo', { word: true, scope: 'repo' }); + expect(api.search).toHaveBeenCalledWith('foo', { word: true, scope: 'repo', content: 'full' }); let s = useStore.getState(); expect(s.search.kind).toBe('word'); expect(s.search.direction).toBe(1); @@ -1505,7 +1505,7 @@ describe('symbol navigation', () => { useStore.getState().closeSearch(); }); - it('/ searches only the file the reader is in; g/ widens to the diff without forgetting a repo choice', async () => { + it('/ pins the current file and remembers content modes independently from global search', async () => { ready(); useStore.setState({ activePath: 'b.py' }); api.search.mockResolvedValue({ query: 'x', matches: [{ path: 'b.py', line: 2, text: 'y = 2' }], truncated: false }); @@ -1522,7 +1522,13 @@ describe('symbol navigation', () => { useStore.setState({ activePath: 'a.py' }); await useStore.getState().runSearch('y'); await new Promise((r) => setTimeout(r, 0)); - expect(api.search).toHaveBeenCalledWith('y', { ignoreCase: true, regex: false, scope: 'file', path: 'b.py' }); + expect(api.search).toHaveBeenCalledWith('y', { + ignoreCase: true, + regex: false, + scope: 'file', + content: 'diff', + path: 'b.py', + }); let s = useStore.getState(); expect(s.search.path).toBe('b.py'); expect(s.search.input).toBe('y'); @@ -1530,16 +1536,39 @@ describe('symbol navigation', () => { expect(s.search.index).toBe(0); expect(s.selection?.range.end).toBe(2); - // The remembered scope: a file search leaves the codebase choice for g/ to return to. - useStore.getState().setSearchOptions({ scope: 'repo' }); + useStore.getState().setSearchOptions({ content: 'full' }); await new Promise((r) => setTimeout(r, 0)); - expect(api.search).toHaveBeenLastCalledWith('y', { ignoreCase: true, regex: false, scope: 'repo' }); - expect(useStore.getState().search.path).toBeNull(); - useStore.getState().openSearch('file'); - useStore.getState().openSearch(); + expect(api.search).toHaveBeenLastCalledWith('y', { + ignoreCase: true, + regex: false, + scope: 'file', + content: 'full', + path: 'b.py', + }); + expect(useStore.getState().search.path).toBe('b.py'); expect(useStore.getState().search.scope).toBe('file'); + useStore.getState().openSearch('diff'); + await useStore.getState().runSearch('y'); + expect(api.search).toHaveBeenLastCalledWith('y', { + ignoreCase: true, + regex: false, + scope: 'diff', + content: 'diff', + }); + useStore.getState().setSearchOptions({ content: 'full' }); + await new Promise((r) => setTimeout(r, 0)); + expect(api.search).toHaveBeenLastCalledWith('y', { + ignoreCase: true, + regex: false, + scope: 'diff', + content: 'full', + }); + useStore.getState().openSearch('file'); + expect(useStore.getState().search.content.file).toBe('full'); + useStore.getState().setSearchOptions({ content: 'diff' }); + expect(useStore.getState().search.content.diff).toBe('full'); useStore.getState().closeSearch(); - useStore.setState((s) => ({ search: { ...s.search, scope: 'diff' } })); // the scope outlives the bar; later tests expect the default + useStore.setState((s) => ({ search: { ...s.search, scope: 'diff', content: { file: 'diff', diff: 'diff' } } })); }); it('a file search with no file to pin to says so instead of querying', async () => { @@ -1675,6 +1704,24 @@ describe('request ownership', () => { expect(s.selection?.range.end).toBe(2); }); + it('opening local search drops a pending global result without moving the cursor', async () => { + ready(); + useStore.getState().openSearch('diff'); + const slow = deferred(); + api.search.mockReturnValueOnce(slow.promise); + const run = useStore.getState().runSearch('foo'); + useStore.getState().openSearch('file'); + const selection = useStore.getState().selection; + slow.resolve(hit('b.py', 2)); + await run; + expect(useStore.getState().search.scope).toBe('file'); + expect(useStore.getState().search.matches).toEqual([]); + expect(useStore.getState().search.loading).toBe(false); + expect(useStore.getState().selection).toBe(selection); + useStore.getState().closeSearch(); + useStore.setState((s) => ({ search: { ...s.search, scope: 'diff' } })); + }); + it('a word search completing after a mode switch is dropped', async () => { ready(); lspTarget.focus({ path: 'a.py', side: 'new', line: 3, col: 4, text: 'foo' }); diff --git a/test/client/useKeymap.test.ts b/test/client/useKeymap.test.ts index d42f8f8..d870142 100644 --- a/test/client/useKeymap.test.ts +++ b/test/client/useKeymap.test.ts @@ -166,7 +166,7 @@ describe('useKeymap', () => { } }); - it('/ opens the file-scoped search, g/ the widened one, and gf the tree filter', () => { + it('/ opens the file-scoped search, g/ the global one, and gf the tree filter', () => { const openSearch = vi.fn(); const tree = { openSearch: vi.fn(), getSearchValue: () => '' }; useStore.setState({ openSearch, treeModel: tree as never }); @@ -177,10 +177,10 @@ describe('useKeymap', () => { press('g'); press('/'); expect(openSearch).toHaveBeenLastCalledWith('diff'); - useStore.setState({ search: { ...useStore.getState().search, scope: 'repo' } }); + useStore.setState({ search: { ...useStore.getState().search, scope: 'diff' } }); press('g'); press('/'); - expect(openSearch).toHaveBeenLastCalledWith('repo'); + expect(openSearch).toHaveBeenLastCalledWith('diff'); expect(tree.openSearch).not.toHaveBeenCalled(); press('g'); press('f'); diff --git a/test/git/GitRepo.test.ts b/test/git/GitRepo.test.ts index 2ccf575..48f93f2 100644 --- a/test/git/GitRepo.test.ts +++ b/test/git/GitRepo.test.ts @@ -213,6 +213,20 @@ describe('GitRepo', () => { expect(cut.truncated).toBe(true); }); + it('applies visible line ranges before counting and truncating matches', async () => { + const ranges = new Map([['a.txt', [[3, 4]]]]); + const exact = await repo.grep('o', 'worktree', 1, { paths: ['a.txt'], ranges }); + expect(exact.matches.map((m) => m.line)).toEqual([4]); + expect(exact.truncated).toBe(false); + ranges.set('a.txt', [[2, 4]]); + const cut = await repo.grep('o', 'worktree', 1, { paths: ['a.txt'], ranges }); + expect(cut.matches.map((m) => m.line)).toEqual([2]); + expect(cut.truncated).toBe(true); + const committed = await repo.grep('o', 'feat', 1, { paths: ['a.txt'], ranges }); + expect(committed.matches.map((m) => m.line)).toEqual([2]); + expect(committed.truncated).toBe(false); + }); + it('whole-word search matches complete, case-sensitive words only', async () => { const word = await repo.grep('one', 'worktree', 50, { word: true }); expect(word.matches.map((m) => [m.path, m.line])).toEqual([['a.txt', 1]]); diff --git a/test/server/Server.test.ts b/test/server/Server.test.ts index 5f57ed1..a53afd7 100644 --- a/test/server/Server.test.ts +++ b/test/server/Server.test.ts @@ -148,6 +148,7 @@ describe('Server', () => { it.each([ '/api/file?path=a.txt&rev=other', '/api/search?scope=other', + '/api/search?content=other', '/api/search?word=true', '/api/threads?state=other', '/api/threads/export?state=other', @@ -325,7 +326,7 @@ describe('Server', () => { } }); - it('GET /api/search covers only the diff unless scope=repo, or one file with scope=file', async () => { + it('GET /api/search covers changed files or one file, with full contents requested explicitly', async () => { await writeFile(join(dir, 'a.txt'), 'a\nneedle\n'); await writeFile(join(dir, 'n.txt'), 'needle\n'); // n.txt is committed, so it is in the tree but not in the working diff. @@ -337,8 +338,8 @@ describe('Server', () => { (JSON.parse((await send('GET', `/api/search?${qs}`)).body).matches as { path: string }[]).map((m) => m.path); expect(await paths('q=needle')).toEqual(['a.txt']); expect(await paths('q=needle&scope=diff')).toEqual(['a.txt']); - expect(await paths('q=needle&scope=repo')).toEqual(['a.txt', 'n.txt']); - expect(await paths('q=needle&scope=file&path=n.txt')).toEqual(['n.txt']); + expect(await paths('q=needle&scope=repo&content=full')).toEqual(['a.txt', 'n.txt']); + expect(await paths('q=needle&scope=file&content=full&path=n.txt')).toEqual(['n.txt']); // A file outside the new side matches nothing rather than widening to everything. expect(await paths('q=needle&scope=file&path=missing.txt')).toEqual([]); expect(await paths('q=needle&scope=file')).toEqual([]); @@ -348,6 +349,37 @@ describe('Server', () => { } }); + it('searches diff hunks with context or full changed files independently of file scope', async () => { + const original = Array.from({ length: 620 }, (_, i) => `needle ${i + 1}\n`).join(''); + await writeFile(join(dir, 'context.txt'), original); + execFileSync('git', ['add', 'context.txt'], { cwd: dir, env }); + execFileSync('git', ['commit', '-q', '-m', 'search context fixture'], { cwd: dir, env }); + await writeFile(join(dir, 'context.txt'), original.replace('needle 610\n', 'needle changed\n')); + await session.refresh(); + try { + const search = async (options: string) => { + const response = await send('GET', `/api/search?q=needle&${options}`); + expect(response.status).toBe(200); + return JSON.parse(response.body) as { matches: { path: string; line: number }[]; truncated: boolean }; + }; + for (const scope of ['scope=diff', 'scope=file&path=context.txt']) { + const visible = await search(`${scope}&content=diff`); + expect(visible.matches.map((m) => m.line)).toEqual([607, 608, 609, 610, 611, 612, 613]); + expect(visible.truncated).toBe(false); + const full = await search(`${scope}&content=full`); + expect(full.matches).toHaveLength(500); + expect(full.matches.every((m) => m.path === 'context.txt')).toBe(true); + expect(full.matches[0]?.line).toBe(1); + expect(full.truncated).toBe(true); + } + expect((await search('scope=file&path=n.txt&content=diff')).matches).toEqual([]); + expect((await search('scope=file&path=n.txt&content=full')).matches).toHaveLength(1); + } finally { + await writeFile(join(dir, 'context.txt'), original); + await session.refresh(); + } + }); + it.each(['content-length', 'chunked'])('rejects oversized %s bodies with 413', async (framing) => { const before = config.get(); const body = JSON.stringify({ autoViewed: ['x'.repeat(2 * 1024 * 1024)] });