Feat: Support basic Symbol navigation inside AI coding session#258
Feat: Support basic Symbol navigation inside AI coding session#258
Conversation
This reverts commit b205107.
There was a problem hiding this comment.
Pull request overview
Adds basic symbol linkification + navigation to AI session buffers, building on the existing in-repo file/URL link support so users can jump from agent output to relevant code locations more directly.
Changes:
- Linkify “symbol-like” tokens appearing near a resolved file link and make them clickable with dedicated keybindings.
- Add navigation commands to open the associated file link and then resolve the symbol via xref, helm-gtags, or buffer search.
- Extend
ai-code--explain-diredto include context for multiple selected files/directories.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
ai-code-session-link.el |
Adds symbol candidate detection, properties/keymap, and navigation commands integrated into session linkification. |
test/test_ai-code-session-link.el |
Adds ERT coverage for symbol linkification behavior and symbol navigation fallbacks. |
ai-code-discussion.el |
Updates Dired explain prompt generation to handle multiple selections and format multi-file context. |
| (let* ((file-at-point (dired-get-filename nil t)) | ||
| (git-relative-path (when file-at-point | ||
| (car (ai-code--get-git-relative-paths (list file-at-point))))) | ||
| (files-context-string (when git-relative-path | ||
| (concat "\nFiles:\n@" git-relative-path))) | ||
| (all-marked (dired-get-marked-files)) | ||
| (has-marked-files (> (length all-marked) 1)) | ||
| (context-files (if has-marked-files | ||
| all-marked | ||
| (when file-at-point | ||
| (list file-at-point)))) |
There was a problem hiding this comment.
dired-get-marked-files is now called unguarded. In Dired, this can signal an error when point is not on a file line (e.g., header/subdir lines), which would regress the previous behavior that handled nil from dired-get-filename gracefully. Consider wrapping the call in ignore-errors and/or using the existing “explicit marks” detection pattern used elsewhere (e.g., treat a single entry equal to file-at-point as ‘no explicit marks’) so the command doesn’t error and also supports a single explicitly-marked file.
|
|
||
| (declare-function ai-code-session-navigate-link-at-mouse "ai-code-input" (event)) | ||
| (declare-function ai-code-session-navigate-link-at-point "ai-code-input" ()) | ||
| (declare-function helm-gtags-find-tag "ext:helm-gtags" (tagname)) |
There was a problem hiding this comment.
The declare-function for helm-gtags-find-tag points at "ext:helm-gtags", but there is no corresponding library file in the repo and this doesn’t match typical declare-function usage (it’s usually the providing library name like "helm-gtags"). Using a non-existent file string can confuse tooling (e.g., check-declare/jump-to-definition). Consider changing it to the actual library name or removing the declaration and relying on the existing (fboundp ...) checks.
| (declare-function helm-gtags-find-tag "ext:helm-gtags" (tagname)) | |
| (declare-function helm-gtags-find-tag "helm-gtags" (tagname)) |
| (ert-deftest ai-code-session-link-test-linkify-session-region-prefilters-java-camelcase-symbols () | ||
| "Linkify nearby bare CamelCase symbols for Java files." | ||
| (let* ((root (make-temp-file "ai-code-session-links-java-camelcase-" t)) |
There was a problem hiding this comment.
This test name/docstring suggests the behavior is restricted to Java files, but the implementation linkifies CamelCase candidates regardless of the referenced file extension (and there’s another test explicitly asserting it works outside Java). Consider renaming the test to reflect “Java-style CamelCase symbols” rather than implying a .java-only filter to avoid misleading future readers.
| (ert-deftest ai-code-session-link-test-linkify-session-region-prefilters-python-snake-case-symbols () | ||
| "Linkify nearby bare snake_case symbols for Python files." | ||
| (let* ((root (make-temp-file "ai-code-session-links-python-snake-case-" t)) |
There was a problem hiding this comment.
This test name/docstring suggests snake_case linkification is specific to Python files, but the implementation treats snake_case as a generic symbol candidate (and there’s another test asserting it works outside Python). Consider renaming to “Python-style snake_case symbols” (or similar) so the test description matches the actual behavior.
| (ert-deftest ai-code-session-link-test-linkify-session-region-prefilters-elisp-hyphen-symbols () | ||
| "Linkify code-like Elisp symbols while skipping nearby prose hyphen words." | ||
| (let* ((root (make-temp-file "ai-code-session-links-elisp-symbols-" t)) |
There was a problem hiding this comment.
The term “prefilters Elisp symbols” in this test name/docstring can read like the behavior is gated by file type, but the current implementation applies the same heuristics regardless of the linked file’s extension. Consider renaming to something like “hyphenated elisp-style symbols” / “elisp-symbol-like hyphen candidates” to better match what’s being asserted.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03e1ed97c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (all-marked (dired-get-marked-files)) | ||
| (has-marked-files (> (length all-marked) 1)) | ||
| (context-files (if has-marked-files |
There was a problem hiding this comment.
Treat a single Dired mark as selected context
ai-code--explain-dired only treats marks as active when > 1 entries are returned, so an explicit single marked file is ignored and the command falls back to file-at-point. In Dired, users commonly mark exactly one file and move point elsewhere before running commands, so this now explains the wrong file/directory in that workflow. Using the same “truly marked” logic as ai-code--ask-question-dired would avoid this regression.
Useful? React with 👍 / 👎.
| (when-let* ((link (ai-code-session-link--parse-file-link-text text)) | ||
| (file (plist-get link :file)) | ||
| (abs-file (ai-code-session-link--resolve-session-file file))) | ||
| (find-file-other-window abs-file) |
There was a problem hiding this comment.
Disambiguate duplicate filenames before opening symbol context
Symbol navigation opens the associated file via ai-code-session-link--resolve-session-file, which resolves basename collisions by taking the first match, so in repos with duplicate filenames (for example multiple utils.py files) clicking a symbol can jump into the wrong file and then run xref/search there. Regular file-link navigation already handles this by prompting when multiple candidates exist, so this path should use equivalent disambiguation instead of silently picking one match.
Useful? React with 👍 / 👎.
This is an improvement on top of existing file path & url navigation #249
It will try 1. LSP; 2. xref; 3. helm-gtags to navigate
Verified that both vterm and eat backends works.