Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdd helpers to locate ctag regex delimiter spans and strip leading/trailing regex anchors; apply anchor stripping to tag construction and display; add a treesitter-aware ctag line parser into defaults; broaden tsinjector's line_parser return to accept structured info and prefer provided column bounds. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant UI as "fzf UI / Caller"
end
rect rgba(200,255,200,0.5)
participant TSInject as "win/tsinjector.attach"
end
rect rgba(255,200,200,0.5)
participant LP as "line_parser (defaults ctag parser)"
end
rect rgba(255,255,200,0.5)
participant Utils as "utils (ctag_match / regex_strip_anchors)"
end
UI->>TSInject: request lines for buffer
TSInject->>LP: call line_parser(line)
LP->>Utils: call ctag_match(pattern) if pattern present
Utils-->>LP: start_col, end_col (or nil)
LP->>Utils: regex_strip_anchors(tag)
Utils-->>LP: stripped_tag, start_flag, end_flag
LP-->>TSInject: return text OR { text, start_col?, end_col?, _ft? }
TSInject->>TSInject: normalize to info, clamp/offset columns
TSInject->>UI: emit normalized line + column metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lua/fzf-lua/defaults.lua`:
- Around line 1373-1375: The _treesitter matcher currently runs
line:match("/(.*)/") on the full rendered row which can capture slashes from
file paths; change it to extract just the excmd field (the btags/excmd column)
from the line first (e.g., split or pattern to get the excmd between the tag
columns) and then call utils.regex_strip_anchors on excmd:match("/(.*)/"); keep
the same return signature and nil second value, only alter the input to
utils.regex_strip_anchors so treesitter regexes are parsed from the excmd field
rather than the entire line.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12f06c47-7567-4f3f-a325-5536d6d706f8
📒 Files selected for processing (4)
lua/fzf-lua/defaults.lualua/fzf-lua/make_entry.lualua/fzf-lua/path.lualua/fzf-lua/utils.lua
ibhagwan
force-pushed
the
feat/tags_treesitter
branch
from
350bc9b to
268f108
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lua/fzf-lua/utils.lua (1)
257-266: Minor typo in comment.Line 258: "conver" should be "convert".
✏️ Fix typo
---Remove ^ $ regex anchors if they appear at start/end respectively --- used with ctags to conver the tag to a valid code block +-- used with ctags to convert the tag to a valid code block ---@param s string?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/fzf-lua/utils.lua` around lines 257 - 266, Update the function comment for M.regex_strip_anchors to fix the typo: change "conver" to "convert" in the descriptive line so the comment reads that ctags is used to convert the tag to a valid code block; keep the rest of the docstring and parameter/return annotations unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lua/fzf-lua/defaults.lua`:
- Around line 1373-1382: The _treesitter extractor is including the opening '/'
in the substring; call utils.ctag_match in _treesitter, then extract the regex
body with line:sub(start_col + 2, end_col) (so you skip both delimiters) and
pass that to utils.regex_strip_anchors; also adjust the returned highlight
bounds to skip the delimiter characters (e.g. return start_col = start_col + 1
and end_col = end_col - 1) so the highlighted region matches the extracted text;
update references to utils.ctag_match, _treesitter, and
utils.regex_strip_anchors accordingly.
---
Nitpick comments:
In `@lua/fzf-lua/utils.lua`:
- Around line 257-266: Update the function comment for M.regex_strip_anchors to
fix the typo: change "conver" to "convert" in the descriptive line so the
comment reads that ctags is used to convert the tag to a valid code block; keep
the rest of the docstring and parameter/return annotations unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1402fa61-ba5a-4b59-add7-636d7d0bdd59
📒 Files selected for processing (5)
lua/fzf-lua/defaults.lualua/fzf-lua/make_entry.lualua/fzf-lua/path.lualua/fzf-lua/utils.lualua/fzf-lua/win/tsinjector.lua
✅ Files skipped from review due to trivial changes (1)
- lua/fzf-lua/make_entry.lua
🚧 Files skipped from review as they are similar to previous changes (1)
- lua/fzf-lua/path.lua
ibhagwan
force-pushed
the
feat/tags_treesitter
branch
from
268f108 to
80f2140
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lua/fzf-lua/win/tsinjector.lua (1)
202-203: Assertion is always true given the short-circuit at line 189.Since
ft_bufnris explicitly set tonilwhen_ftis truthy (line 189), this assertion can never fail. It's fine as defensive documentation, but a comment explaining the invariant might be clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/fzf-lua/win/tsinjector.lua` around lines 202 - 203, The assertion assert(not _ft or not ft_bufnr) is redundant because ft_bufnr is already set to nil when _ft is truthy (in the code that sets ft_bufnr), so replace the assert with a brief explanatory comment documenting the invariant (e.g., "_ft truthy implies ft_bufnr is nil, maintained above") or remove the assertion entirely; update the code around the symbols _ft and ft_bufnr in tsinjector.lua to reflect this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lua/fzf-lua/win/tsinjector.lua`:
- Around line 202-203: The assertion assert(not _ft or not ft_bufnr) is
redundant because ft_bufnr is already set to nil when _ft is truthy (in the code
that sets ft_bufnr), so replace the assert with a brief explanatory comment
documenting the invariant (e.g., "_ft truthy implies ft_bufnr is nil, maintained
above") or remove the assertion entirely; update the code around the symbols _ft
and ft_bufnr in tsinjector.lua to reflect this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75ccd033-9fad-467f-8d8c-86f0ca440887
📒 Files selected for processing (7)
.emmyrc.jsonlua/fzf-lua/defaults.lualua/fzf-lua/make_entry.lualua/fzf-lua/path.lualua/fzf-lua/utils.lualua/fzf-lua/win/tsinjector.luatests/utils_spec.lua
✅ Files skipped from review due to trivial changes (2)
- .emmyrc.json
- lua/fzf-lua/make_entry.lua
🚧 Files skipped from review as they are similar to previous changes (2)
- lua/fzf-lua/utils.lua
- lua/fzf-lua/path.lua
ibhagwan
force-pushed
the
feat/tags_treesitter
branch
3 times, most recently
from
f5fc9ca to
5568be7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lua/fzf-lua/win/tsinjector.lua (1)
175-223:⚠️ Potential issue | 🟠 MajorRebase parser-supplied columns onto the full line.
Line 175 passes
line:sub(min_col)toline_parser, soinfo.start_col/info.end_colare slice-local. Lines 220-223 then treat them as full-line columns. With left-side preview layouts (min_col > 0), the new structured_treesitterpath returns regions that are shifted left or collapsed, so highlighting lands on the wrong text or disappears.🐛 Proposed fix
- local filepath, _lnum, info, _ft = line_parser(line:sub(min_col)) + local line_slice = line:sub(min_col) + local col_offset = `#line` - `#line_slice` + local filepath, _lnum, info, _ft = line_parser(line_slice) -- info can be a string or `{ text = ..., start_col, end_col }` info = type(info) == "table" and info or { text = info } @@ - local start_col = math.max(min_col, info.start_col or (line_len - `#text`)) - local end_col = max_col - and math.min(max_col, info.end_col or line_len) - or info.end_col or (line_len - trim_right) + local start_col = info.start_col ~= nil + and (col_offset + info.start_col) + or (line_len - `#text`) + local end_col = info.end_col ~= nil + and (col_offset + info.end_col) + or (line_len - trim_right) + start_col = math.max(min_col, start_col) + if max_col then + end_col = math.min(max_col, end_col) + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lua/fzf-lua/win/tsinjector.lua` around lines 175 - 223, The parser was called with line:sub(min_col) so info.start_col/info.end_col are relative to that slice; rebase them onto the full line before using (e.g. add min_col-1 when info.start_col/info.end_col exist) where start_col/end_col are computed in the block that currently calculates start_col and end_col; ensure the math handles nils (use info.start_col/info.end_col only when present) and preserves existing fallbacks (line_len - `#text`, trim_right, max_col logic) and keep the assert around _ft/ft_bufnr unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lua/fzf-lua/utils.lua`:
- Around line 273-295: The rfind_slash helper inside M.ctag_match incorrectly
treats a slash as escaped by only inspecting the single preceding character;
update rfind_slash to, when it sees a slash (SLASH), walk backward counting the
contiguous run of backslashes (BSLASH) immediately before that slash and treat
the slash as escaped only if that run length is odd; ensure rfind_slash still
returns the index of the unescaped slash (or nil), and leave the outer logic
using rfind_slash (start_col, end_col, and the second call rfind_slash(str,
end_col - 1)) unchanged so ctag_match correctly handles sequences like
`/foo\\/`.
---
Outside diff comments:
In `@lua/fzf-lua/win/tsinjector.lua`:
- Around line 175-223: The parser was called with line:sub(min_col) so
info.start_col/info.end_col are relative to that slice; rebase them onto the
full line before using (e.g. add min_col-1 when info.start_col/info.end_col
exist) where start_col/end_col are computed in the block that currently
calculates start_col and end_col; ensure the math handles nils (use
info.start_col/info.end_col only when present) and preserves existing fallbacks
(line_len - `#text`, trim_right, max_col logic) and keep the assert around
_ft/ft_bufnr unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e69d778f-87fe-436f-a803-66cda172aa85
📒 Files selected for processing (7)
.emmyrc.jsonlua/fzf-lua/defaults.lualua/fzf-lua/make_entry.lualua/fzf-lua/path.lualua/fzf-lua/utils.lualua/fzf-lua/win/tsinjector.luatests/utils_spec.lua
✅ Files skipped from review due to trivial changes (2)
- lua/fzf-lua/make_entry.lua
- .emmyrc.json
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/utils_spec.lua
ibhagwan
force-pushed
the
feat/tags_treesitter
branch
4 times, most recently
from
e8ae528 to
8a1e18f
Compare
Merged
ibhagwan
force-pushed
the
feat/tags_treesitter
branch
6 times, most recently
from
7320aa2 to
a23c05a
Compare
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lua/fzf-lua/defaults.lua`:
- Around line 1333-1343: The filename extraction fails because start_col has
already advanced past the opening '/' so the substring still contains that
slash; update the substring used to build the prefix in the anonymous file
function (the line:sub(...) call that currently uses start_col - had_caret - 1)
to exclude the raw slash (use start_col - had_caret - 2) so the
match("([^%s]+%.[^%s]+)%s+$") can find the filename; keep the include_file guard
and the subsequent "if not file return" behavior.
In `@lua/fzf-lua/utils.lua`:
- Around line 261-268: The helper M.regex_strip_anchors currently always strips
a trailing '$', corrupting patterns like "foo\$"; update M.regex_strip_anchors
so it only removes the final '$' when it is not escaped: when string.byte(s, `#s`)
== 36, count the consecutive backslashes immediately preceding that '$' and only
treat the '$' as an anchor (remove it and set had_dollar=1) if the backslash
count is even (i.e., the '$' is not escaped); keep behavior for the leading '^'
as-is. This change fixes returned text for callers like entry_to_ctag and
ctag_line_parser which rely on unmodified escaped terminal dollars.
In `@lua/fzf-lua/win/tsinjector.lua`:
- Around line 175-177: The left-trim uses a 1-based Lua substring but min_col is
a 0-based display offset, so change the slice to start at min_col+1 (i.e.
replace line:sub(min_col) with line:sub(min_col + 1)) so the parser
(line_parser) sees no hidden byte; verify col_offset (currently computed as
`#line` - `#line_slice`) now equals min_col and that downstream structured
start_col/end_col calculations align correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df19c110-5b60-4cbd-9ab8-af734341931f
📒 Files selected for processing (8)
.emmyrc.jsonlua/fzf-lua/defaults.lualua/fzf-lua/make_entry.lualua/fzf-lua/path.lualua/fzf-lua/types.lualua/fzf-lua/utils.lualua/fzf-lua/win/tsinjector.luatests/utils_spec.lua
✅ Files skipped from review due to trivial changes (3)
- lua/fzf-lua/make_entry.lua
- .emmyrc.json
- tests/utils_spec.lua
ibhagwan
force-pushed
the
feat/tags_treesitter
branch
2 times, most recently
from
31cc585 to
63bb956
Compare
ibhagwan
force-pushed
the
feat/tags_treesitter
branch
from
63bb956 to
1410eab
Compare
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Style
Tests