Skip to content

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

Merged
ghackett merged 2 commits into
mainfrom
bare-root-file-links
Aug 5, 2026
Merged

feat(terminal): bare root-level filenames are clickable links#177
ghackett merged 2 commits 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 — the fallback is deliberate: in that case the whole tab, editor and quick open included, is already rooted at home);
  • a Gio.FileMonitor on the root rebuilds the tag when the name set changes. Change events coalesce on a 500ms timer armed by the first event — a leading-edge throttle rather than a trailing-edge debounce, so steady churn can't starve the refresh; content-only writes never churn the tag;
  • 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) — these run on CI.
  • scripts/check_root_name_links.py (committed) covers the stateful side CI can't reach: map-time root resolution against a real TerminalTab, tag registration, monitor-driven rebuild on a created file, no rebuild on content-only writes, and monitor teardown. It's a script rather than a pytest test because tests/conftest.py deliberately blocks the GTK namespaces suite-wide so local runs reproduce CI (which has no gir packages or display). Run it on a dev machine; it shows no window and spawns no agent.
  • Generated patterns (including unicode and metacharacter names) verified to compile under VTE's actual 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

@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)

@ghackett
ghackett marked this pull request as ready for review August 5, 2026 17:40
@ghackett

ghackett commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Addressed the review in 0734a86:

  1. Wiring check committed — now lives at scripts/check_root_name_links.py. It's a script rather than a pytest test because tests/conftest.py blocks the GTK-stack namespaces for the whole suite (so local runs reproduce CI, which installs python3-gi only); per conftest's own guidance, testing widgets for real means running outside the suite. The script drives a real TerminalTab (no window shown, command_override="true" so no agent spawns) and covers map-time root resolution, tag registration, monitor-driven rebuild, the no-rebuild-on-content-write case, and teardown. Verified passing locally from its committed location.

  2. Throttle wording fixed — the class docstring (and the PR body) now call it what it is: a leading-edge throttle that coalesces change events on a 500ms timer armed by the first event. Kept the behavior rather than switching to a trailing-edge debounce, deliberately: an agent churning root files steadily would starve a true debounce indefinitely, while a rebuild that lands mid-burst is harmless (the next event re-arms the timer, and the name-set comparison already swallows no-op refreshes).

  3. HOME fallback is intentional and now documented at the link_root assignment: when the project dir is gone, the whole tab is already rooted at home — the editor, quick open, and click-time resolution — so bare-name links follow suit rather than being the one link kind that goes dark. Scope stays modest in practice: _file_names excludes directories, and a home directory's top level is mostly directories.

  4. CI ran the new bare_names_pattern unit tests on push (green), covering the static-tracing gap noted.

ghackett and others added 2 commits August 5, 2026 13:51
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
…fallback

Addresses the PR 177 review: the _RootNameLinks wiring check now lives at
scripts/check_root_name_links.py (a script, not a pytest test — conftest
blocks the GTK namespaces suite-wide so local runs reproduce CI); the
class docstring now calls the 500ms coalescing what it is, a leading-edge
throttle chosen so steady churn can't starve the refresh; and link_root's
comment spells out that inheriting editor_root's HOME fallback is
deliberate — the whole tab is rooted at home in that case, so bare names
follow the editor and quick open rather than going dark.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KBnWBFL3yHBnJy4kupKZgk
@ghackett
ghackett force-pushed the bare-root-file-links branch from 0734a86 to 7c66860 Compare August 5, 2026 17:52
@ghackett
ghackett merged commit fb6be4d into main Aug 5, 2026
2 checks passed
@ghackett
ghackett deleted the bare-root-file-links branch August 5, 2026 17:53
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