Skip to content

feat(terminal): bare root-level filenames are clickable links - #177

Draft
ghackett wants to merge 1 commit into
mainfrom
bare-root-file-links
Draft

feat(terminal): bare root-level filenames are clickable links#177
ghackett wants to merge 1 commit into
mainfrom
bare-root-file-links

Conversation

@ghackett

@ghackett ghackett commented Aug 3, 2026

Copy link
Copy Markdown
Member

Problem

Clickable file references (#173) require a slash: collins/foo.py:12 underlines, but root-level references like README.md or pyproject.toml — which Claude emits constantly — stay inert. The slash requirement exists for a good reason: a bare-filename shape (\w+\.\w+) would underline e.g., Node.js, and 1.2.3 in ordinary prose, and VTE has no per-match callback that would let the hover underline consult the filesystem.

Approach

Match the actual names at the project root instead of a shape. linkpatterns.bare_names_pattern(names) builds a PCRE2/re-shared alternation of literal names with the same boundary discipline as FILE_PATTERN:

  • boundary (or line start) before, optional :line[:col] suffix after;
  • the token must not continue with a character a path could end on, so README.md. sheds its sentence period while README.mdx never half-matches an entry README.md;
  • longest name first, so README.md.bak beats its own prefix;
  • names containing whitespace, : or / are unboundable/ambiguous and stay out (as agreed, spaces remain out of scope).

_RootNameLinks keeps the tag registered on each terminal (agent tab and shell-panel tabs alike):

  • built on first map from the tab's new link_root — the directory the editor opens at (the tab's cwd, falling back to home), kept even when GtkSourceView is missing;
  • a Gio.FileMonitor on the root rebuilds the tag when the name set changes (debounced 500ms; content-only writes don't churn it), closing the staleness gap;
  • files only — root directory names (docs, tests) are everyday prose words, and docs/ already belongs to the slashed grammar;
  • clicks reuse the existing "file" kind end-to-end: resolve_file_reference already handles bare relative candidates, so images still hit the lightbox and text files open in the editor at the referenced line.

Performance: VTE evaluates match regexes only against the row under the pointer, and PCRE2 compiles a literal alternation efficiently — a typical root is 15–40 entries, a few hundred bytes of pattern.

Testing

  • New GTK-free unit tests for bare_names_pattern (matching, suffixes, trailing-punctuation shedding, half-match boundaries, longest-first ordering, metacharacter escaping, unboundable-name filtering).
  • Headless wiring check against a real TerminalTab: map-time root resolution and tag registration, monitor-driven rebuild on a created file, no rebuild on content-only writes, monitor teardown on destroy — all pass.
  • Generated patterns (including unicode and metacharacter names) verified to compile under VTE's PCRE2, not just Python re.

No screenshots: the change is a hover underline over live terminal rows, which an in-process snapshot can't show without synthetic pointer motion.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KBnWBFL3yHBnJy4kupKZgk

The path grammar demands a slash, so root-level references (README.md,
pyproject.toml) never underlined. A shape can't take them — bare-filename
shapes over-match prose — but an alternation of the names actually sitting
at the project root can: the underline then only ever lands on a name that
exists. bare_names_pattern builds that grammar (same boundary discipline
and :line[:col] suffix as FILE_PATTERN, longest name first so an entry
extending another wins), and _RootNameLinks keeps it registered per
terminal — built on first map from the tab's link_root (the directory the
editor opens at), rebuilt via a debounced directory monitor when the
root's name set changes. Files only: root directory names are everyday
prose words, and `docs/` already belongs to the path grammar.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KBnWBFL3yHBnJy4kupKZgk
@ghackett

ghackett commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @ghackett's task in 3m 50s —— View job


Review complete

  • Gather context (PR diff, repo CLAUDE.md)
  • Read changed files (linkpatterns.py, terminal.py, test_linkpatterns.py)
  • Review code for correctness, security, performance, style
  • Post review feedback

(No CLAUDE.md exists in this repo, so no repo-specific conventions to check against beyond what's visible in the code itself.)

Nicely done overall — the regex reasoning is careful and the docstrings actually explain the why (boundary discipline, longest-first ordering, the name-set-not-monitor-event debounce guard). I traced bare_names_pattern's lookaround logic by hand against every parametrized case in the new tests (trailing punctuation shedding, README.mdx non-half-match, README.md.bak prefix ordering, metacharacter escaping) and it holds up. A few things worth a look:

1. The PR body's testing claims outrun what's actually in the diff.
The description says a "headless wiring check against a real TerminalTab" verifies map-time root resolution, monitor-driven rebuild, no-rebuild-on-content-write, and monitor teardown on destroy — but changed_files only shows tests/test_linkpatterns.py touched, and there's no tests/test_terminal.py in the repo at all. tests/conftest.py deliberately blocks the Vte/Gtk namespaces on CI (_CI_MISSING_NAMESPACES), so this is plausibly a real check that was run manually and just isn't committed anywhere — which is consistent with the repo's constraints, but it means the most stateful part of this PR (_RootNameLinks's map lifecycle, 500ms debounce, monitor teardown in collins/terminal.py:267-320) has no regression coverage going forward. Worth either committing that wiring check somewhere (even if skipped on CI) or being explicit in the PR that it's manual-only verification.

2. "Debounced 500ms" is actually a leading-edge throttle, not a debounce.
_on_root_changed (collins/terminal.py:283-285) only schedules a refresh if none is already pending — it doesn't push the deadline back on each new event. So a burst of directory changes fires the rebuild ~500ms after the first event, not 500ms after the last one. Functionally fine (the name-set comparison in _apply makes a stale-then-refreshed rebuild harmless), but the docstring/PR description language ("debounced") implies trailing-edge behavior that isn't what's implemented. Minor, but worth a comment tweak or rename to avoid confusion for the next reader.

3. Home-directory fallback widens _RootNameLinks's scope beyond "a project root."
link_root (collins/terminal.py:1074-1078) inherits editor_root's existing fallback: if cwd is missing/not a directory, the root becomes Path.home(). That's pre-existing behavior for the editor pane, but this PR now also uses it to scandir/monitor and build a bare-name alternation — so in that edge case every terminal row would light up hover-links for whatever sits at the top of $HOME (dotfiles included, since _file_names doesn't filter hidden entries). Probably rare in practice (cwd is usually valid), but the "15-40 entries" performance framing in the PR body assumes a project root, not $HOME, so it's worth confirming this fallback is intentional for this feature specifically.

4. Minor: I wasn't able to actually execute pytest tests/test_linkpatterns.py in this sandbox (Bash execution was declined), so the above is based on careful static tracing of the regex/state-machine logic against the new test cases, not a live run. The logic checks out by hand, but if you want a second confirmation, CI should exercise this on push.

Nothing here blocks merging — #1 and #3 are worth a sentence of clarification, #2 is a wording nit.

Fix debounce wording →
(branch)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant