Repository navigation
fix(tui): add readdir fallback for @-mention in non-git directories (#266) #268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
f9c2003
0f3cc36
f565824
cdbd382
3da0d7f
d9eb875
e48823a
4950e9d
5b813e6
f095bba
50ad3ef
fbde230
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| `@` file completion now works in non-git directories. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,11 +26,14 @@ | |
| * | ||
| * When `fd` is available the inner pi-tui provider owns the `@` branch | ||
| * verbatim — its fd invocation respects `.gitignore` and is strictly | ||
| * better than anything we can cheaply reproduce in TS. We only kick in | ||
| * when `fd` is missing AND we're in a git repo. | ||
| * better than anything we can cheaply reproduce in TS. When `fd` is | ||
| * missing, we only fall back to our own recursive readdir when the | ||
| * work dir is not a git repository; inside a git repo we trust the | ||
| * `git ls-files` snapshot to honor `.gitignore`. | ||
| */ | ||
|
|
||
| import { basename } from 'node:path'; | ||
| import { existsSync, readdirSync, statSync } from 'node:fs'; | ||
| import { basename, join } from 'node:path'; | ||
|
|
||
| import { | ||
| CombinedAutocompleteProvider, | ||
|
|
@@ -46,13 +49,43 @@ import type { GitLsFilesCache, GitSnapshot } from '#/utils/git/git-ls-files'; | |
|
|
||
| const MAX_SUGGESTIONS_WHEN_QUERY = 50; | ||
| const MAX_SUGGESTIONS_WHEN_EMPTY = 15; | ||
| const READDIR_TTL_MS = 2000; | ||
| const READDIR_MAX_ENTRIES = 1000; | ||
| const READDIR_MAX_DEPTH = 8; | ||
|
|
||
| // Directories that are typically too large or too auto-generated to be | ||
| // useful for @-completion. Skipping them keeps the walk snappy on | ||
| // real-world repos that don't have fd or git. | ||
| const SKIP_DIRS = new Set([ | ||
| '.git', | ||
| 'node_modules', | ||
| 'dist', | ||
| 'build', | ||
| '.next', | ||
| '.turbo', | ||
| '.parcel-cache', | ||
| '.cache', | ||
| '__pycache__', | ||
| '.venv', | ||
| 'target', | ||
| '.idea', | ||
| '.vscode', | ||
| ]); | ||
|
|
||
| /** Structurally compatible with `GitSnapshot` so existing rankers accept it. */ | ||
| interface ReadDirSnapshot { | ||
| readonly files: readonly string[]; | ||
| readonly mtimeByPath: ReadonlyMap<string, number>; | ||
| readonly recencyOrder: ReadonlyMap<string, number>; | ||
| } | ||
|
|
||
| // Mirrors pi-tui's PATH_DELIMITERS. Keeping a local copy so @-detection | ||
| // stays aligned even if pi-tui extends its set. | ||
| const PATH_DELIMITERS = new Set([' ', '\t', '"', "'", '=']); | ||
|
|
||
| export class FileMentionProvider implements AutocompleteProvider { | ||
| private readonly inner: CombinedAutocompleteProvider; | ||
| private readonly readDirWalker: ReadDirWalker; | ||
|
|
||
| constructor( | ||
| slashCommands: SlashCommand[], | ||
|
|
@@ -61,6 +94,7 @@ export class FileMentionProvider implements AutocompleteProvider { | |
| private readonly gitCache: GitLsFilesCache, | ||
| ) { | ||
| this.inner = new CombinedAutocompleteProvider(slashCommands, workDir, fdPath); | ||
| this.readDirWalker = new ReadDirWalker(workDir); | ||
| } | ||
|
|
||
| async getSuggestions( | ||
|
|
@@ -85,8 +119,30 @@ export class FileMentionProvider implements AutocompleteProvider { | |
| return this.inner.getSuggestions(lines, cursorLine, cursorCol, options); | ||
| } | ||
|
|
||
| if (!this.gitCache.isGitRepo()) { | ||
| // Not in a git repo (stable for the cache's lifetime — `isGitRepo` | ||
| // is captured at TUI startup by `git rev-parse --show-toplevel`). | ||
| // Transient `git ls-files` failures inside a real repo leave | ||
| // `getSnapshot()` returning null but `isGitRepo()` still true, in | ||
| // which case we deliberately do NOT fall back to raw readdir | ||
| // (that would bypass `.gitignore`). Inner's getFuzzyFileSuggestions | ||
| // is a dead end without `fd`, so we own the candidate source here. | ||
| // See issue #266. | ||
| const readdirResult = this.buildFromReadDir(atPrefix); | ||
| if (readdirResult !== null) { | ||
| return { items: readdirResult, prefix: atPrefix }; | ||
| } | ||
| return this.inner.getSuggestions(lines, cursorLine, cursorCol, options); | ||
| } | ||
| const snapshot = this.gitCache.getSnapshot(); | ||
| if (snapshot === null || snapshot.files.length === 0) { | ||
| if (snapshot === null) { | ||
| // Inside a git repo but the snapshot fetch failed transiently | ||
| // (e.g. `git ls-files` returned non-zero, lock contention, or | ||
| // the index mtime lookup raced). Don't consult raw readdir — | ||
| // it would bypass `.gitignore` and could surface ignored files. | ||
| // Fall through to the inner provider, which can still resolve | ||
| // `/path` or quoted-path completions; on failure it returns | ||
| // null and the editor dismisses the menu. | ||
| return this.inner.getSuggestions(lines, cursorLine, cursorCol, options); | ||
| } | ||
|
|
||
|
|
@@ -102,14 +158,40 @@ export class FileMentionProvider implements AutocompleteProvider { | |
| : rankForQuery(candidates, query, snapshot); | ||
|
|
||
| if (items.length === 0) { | ||
| // Git cache had nothing useful — fall through to readdir (user | ||
| // may be typing a path that exists but isn't tracked, e.g. a | ||
| // freshly created file not yet in the 2s cache). | ||
| // Git ls-files had no match for this query. Inside a git repo we | ||
| // do NOT consult readdir — a recursive readdir would bypass | ||
| // `git ls-files --exclude-standard` and could surface | ||
| // .gitignored paths. Fall through to the inner provider, which | ||
| // can still resolve `/path` or quoted-path completions. | ||
| return this.inner.getSuggestions(lines, cursorLine, cursorCol, options); | ||
| } | ||
| return { items, prefix: atPrefix }; | ||
| } | ||
|
|
||
| private buildFromReadDir(atPrefix: string): AutocompleteItem[] | null { | ||
| const snapshot = this.readDirWalker.getSnapshot(); | ||
| if (snapshot === null || snapshot.files.length === 0) { | ||
| return null; | ||
| } | ||
| const query = atPrefix.slice(1); | ||
| const includeDotDirs = query.startsWith('.'); | ||
| const candidates = includeDotDirs | ||
| ? snapshot.files | ||
| : snapshot.files.filter((p) => !containsDotSegment(p)); | ||
| if (candidates.length === 0) { | ||
| return null; | ||
| } | ||
| const ranked = | ||
| query.length === 0 | ||
| ? rankForEmptyQuery(candidates, snapshot) | ||
| : rankForQuery(candidates, query, snapshot); | ||
| // An empty ranking means the walker saw files but none matched the | ||
| // query. Returning `null` (rather than `{ items: [] }`) lets the | ||
| // caller dismiss the autocomplete menu instead of presenting an | ||
| // empty state. | ||
| return ranked.length === 0 ? null : ranked; | ||
| } | ||
|
|
||
| applyCompletion( | ||
| lines: string[], | ||
| cursorLine: number, | ||
|
|
@@ -150,6 +232,101 @@ function containsDotSegment(path: string): boolean { | |
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Recursive readdir of the work dir, used as the @-completion source | ||
| * when `fd` is missing and we're not in a git repository. Caches the | ||
| * result for `READDIR_TTL_MS` to keep keystroke latency low. Skips | ||
| * well-known build/dependency directories so a `node_modules`-laden | ||
| * repo still walks in under ~50ms. | ||
| * | ||
| * The walker collects dot entries too (so callers can opt in via | ||
| * `@.env` / `@.github/`); the actual dot-filtering is the caller's | ||
| * responsibility, mirroring the git-backed path. | ||
| */ | ||
| class ReadDirWalker { | ||
| private snapshot: ReadDirSnapshot | null = null; | ||
| private fetchedAt = 0; | ||
|
|
||
| constructor(private readonly workDir: string) {} | ||
|
|
||
| getSnapshot(): ReadDirSnapshot | null { | ||
| if (!existsSync(this.workDir)) return null; | ||
| const now = Date.now(); | ||
| if (this.snapshot !== null && now - this.fetchedAt < READDIR_TTL_MS) { | ||
| return this.snapshot; | ||
| } | ||
| const next = this.walk(); | ||
| if (next === null) return null; | ||
| this.snapshot = next; | ||
| this.fetchedAt = now; | ||
| return next; | ||
| } | ||
|
|
||
| private walk(): ReadDirSnapshot | null { | ||
| const files: string[] = []; | ||
| const mtimeByPath = new Map<string, number>(); | ||
| try { | ||
| this.walkDir(this.workDir, '', 0, files, mtimeByPath); | ||
| } catch { | ||
| return null; | ||
| } | ||
| files.sort(); | ||
| const capped = files.length > READDIR_MAX_ENTRIES ? files.slice(0, READDIR_MAX_ENTRIES) : files; | ||
|
zha-ji-tui marked this conversation as resolved.
|
||
| return { files: capped, mtimeByPath, recencyOrder: new Map() }; | ||
| } | ||
|
|
||
| private walkDir( | ||
| absDir: string, | ||
| relDir: string, | ||
| depth: number, | ||
| files: string[], | ||
| mtimeByPath: Map<string, number>, | ||
| ): void { | ||
| if (depth > READDIR_MAX_DEPTH) return; | ||
| if (files.length >= READDIR_MAX_ENTRIES) return; | ||
| let entries: import('node:fs').Dirent[]; | ||
| try { | ||
| entries = readdirSync(absDir, { withFileTypes: true }); | ||
|
zha-ji-tui marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fresh evidence after the previous rebuttal: the current code still calls Useful? React with 👍 / 👎. |
||
| } catch { | ||
| return; | ||
| } | ||
| // Walk visible (non-dot) entries before hidden ones so a hidden | ||
| // subtree like `.config/` cannot exhaust READDIR_MAX_ENTRIES with | ||
| // hidden paths and push a visible file out of the snapshot. Hidden | ||
| // paths are still collected — they fill any remaining capacity, so | ||
| // the opt-in `@.env` / `@.github/` queries still work. | ||
| const ordered = entries.toSorted((a, b) => { | ||
| const aHidden = a.name.startsWith('.') ? 1 : 0; | ||
| const bHidden = b.name.startsWith('.') ? 1 : 0; | ||
| return aHidden - bHidden; | ||
| }); | ||
| for (const entry of ordered) { | ||
| // Short-circuit the loop once the cap is reached. The top-of- | ||
| // function check guards the recursion entry; this one stops the | ||
| // per-entry iteration so a single large directory doesn't | ||
| // statSync every remaining file after the cap is filled. | ||
| if (files.length >= READDIR_MAX_ENTRIES) break; | ||
| if (entry.name === '.' || entry.name === '..') continue; | ||
| if (entry.isDirectory()) { | ||
| if (SKIP_DIRS.has(entry.name)) continue; | ||
| const absChild = join(absDir, entry.name); | ||
| const relChild = relDir === '' ? entry.name : `${relDir}/${entry.name}`; | ||
| this.walkDir(absChild, relChild, depth + 1, files, mtimeByPath); | ||
|
zha-ji-tui marked this conversation as resolved.
Outdated
zha-ji-tui marked this conversation as resolved.
Outdated
|
||
| } else if (entry.isFile()) { | ||
|
zha-ji-tui marked this conversation as resolved.
Outdated
|
||
| const absPath = join(absDir, entry.name); | ||
| const relPath = relDir === '' ? entry.name : `${relDir}/${entry.name}`; | ||
| try { | ||
| const stat = statSync(absPath); | ||
| files.push(relPath); | ||
| mtimeByPath.set(relPath, stat.mtimeMs); | ||
| } catch { | ||
| // File disappeared between readdir and stat — skip it. | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Empty-query ranking: stratified by signal strength. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
fdis missing in a non-git workdir, an explicit query like@.vscode/settingscan never return.vscode/settings.jsonbecause the walker drops.vscodebeforebuildFromReadDircan apply the existingquery.startsWith('.')hidden-path opt-in. This leaves common project config files unmentionable in exactly the fallback path being added; consider only skipping these dirs for the default menu or allowing them for targeted hidden queries.Useful? React with 👍 / 👎.