Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The linking side of the fix works where I could run it, but I couldn't check the directory click itself: it needs the desktop app.
How I checked: I ran opencode serve from source for the base and this PR. Against it I ran each checkout's real inlineCodeKind and checkFileLinkExists, with that checkout's exists/kind logic, on a project with sub/ and my folder/file.txt. With the PR, C:\Program Files\app\file.txt and …/my folder/file.txt are classified as paths, and sub, <abs>/sub and sub/ become links; on the base none of them do. The new tests pass, and typecheck is clean in gui-extensions, session-ui and app. Before/after screenshots or a short recording of clicking a nested folder link in the desktop app would help.
Public API: this adds a required kind(path): Promise<"file" | "directory" | undefined> method to the Files interface in the gui-extensions SDK (sdk/host-apis.ts), and exists() now also returns true for directories. Code that only calls Files is unaffected, but any other implementation of Files must add kind to type-check.
Concerns (details inline):
- A folder click expands only that folder. A nested folder under collapsed parents stays hidden.
- Every file-link click now waits for a directory listing before reading the file.
- Every relative inline-code span the search index doesn't match now also triggers a directory listing. With whitespace now allowed, many more spans qualify.
|
|
||
| // A directory has no tab to open: reveal it in the file tree instead of loading it as a file, | ||
| // which would fail with a not-found toast. | ||
| if ((await files.kind(targetPath)) === "directory") { |
There was a problem hiding this comment.
This adds a file.list round trip before every file-link click, including the common case where the target is a file. Could the directory check run only when it's needed? For example, when sync reports not-found, or by reusing what the search match already tells us (search only returns files).
This only expands the folder itself. expandDir doesn't open its parents, so for packages/app under a collapsed packages the click switches the tree to "all" and nothing visibly changes. Could this expand every parent folder too, and scroll the node into view? Also, files.absolute(targetPath) already exists on Files and covers UNC paths and backslashes; the inline regex here doesn't.
| if (await findFileLink({ files: input.files, path: parsed.path, signal: input.signal })) return true | ||
|
|
||
| // The search index covers files only; a directory named in a message still gets a link. | ||
| return existing(parsed.path) |
There was a problem hiding this comment.
Every relative candidate the search index doesn't match now triggers an extra directory listing (and the abort signal isn't passed to it). I measured one extra request for console.log and for git add src/foo.ts, neither of which links. With the whitespace check gone, many more inline-code spans reach this point (JSON.parse, shell commands, etc.), and the per-render checked set is the only dedupe, so it repeats on every re-render. Could the fallback be limited to paths that can actually name a directory? For example, only paths containing / or ending in /, or with no file extension.
|
Addressed both points: (1) the click path now reuses the directory listing that linkified the path (a small path→kind cache per session root), so a normal file click adds no |
Issue for this PR
Closes #55863
Type of change
What does this PR do?
File links in messages only worked for plain files that the search index knows about:
type === "file", so directories could never become links. A hand-written markdown link to a directory did link, then failed on click with a "File not found" toast because the click path reads the target as a file.C:\Program Files\app\file.txtwere never linkified even though thefile://route linked them.Changes:
Files.kind(path)(new, next toexists) returns"file" | "directory" | undefinedfrom the same directory listing;existskeeps its boolean contract and now accepts directories too.checkFileLinkExistsuseskind, and falls back to it for workspace-relative paths the search index (files only) does not cover, so directories named in a message get links.How did you verify your code works?
bun test src/components/markdown-inline-code-kind.test.ts— 28 pass, includingC:\Program Files\app\file.txtandmy docs/notes.md; these 3 cases fail on base.bun test src/file/resolve-link.test.ts— 32 pass, including absolute and workspace-relative directory links; fails on base.bun test src/file/— 53 pass;bun run typecheckclean ingui-extensionsandsession-ui.packages/apptypecheck cannot run in this Windows checkout:src/custom-elements.d.tsis a git symlink materialized as a text file, sotsc -breports TS1128 before reaching any source. Unrelated to this change.Screenshots / recordings
N/A.
Checklist
Added by the opencode bot (AI-generated).
Related: #37891, #51723, #51872
Public API: adds required
kind(path)to the gui-extensionsFilesinterface;exists()now also returns true for directories (callers unaffected, otherFilesimplementations must addkind)