Skip to content

fix(previewer): rg errmsg is mistakenly taken as ctags pattern#2687

Merged
phanen merged 1 commit intomainfrom
always-clear-match
Apr 20, 2026
Merged

fix(previewer): rg errmsg is mistakenly taken as ctags pattern#2687
phanen merged 1 commit intomainfrom
always-clear-match

Conversation

@phanen
Copy link
Copy Markdown
Collaborator

@phanen phanen commented Apr 20, 2026

Problem:
When rg cmd fail with errmsg e.g. rg: PCRE2: error
This will be parsed and treated as an entry(with lnum=0)

matchadd will remain even buf changed in window, so
we have to clear the matches on each preview.

To reproduce: FzfLua live_grep search=\] no_esc=true
then type <bs>, ], <bs>, ]... (with rg --pcre2)

Maybe this can also happened in FzfLua global.

Summary by CodeRabbit

  • Bug Fixes
    • Improved cursor highlighting accuracy in preview mode for entries with missing line numbers by refining condition logic.

Problem:
When rg cmd fail with errmsg e.g. `rg: PCRE2: error`
This will be parsed and treated as an entry(with lnum=0)

`matchadd` will remain even buf changed in window, so
we have to clear the matches on each preview.

To reproduce: `FzfLua live_grep search=\] no_esc=true`
then type `<bs>`, `]`, `<bs>`, `]`... (with rg --pcre2)

Maybe this can also happened in `FzfLua global`.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 20, 2026

📝 Walkthrough

Walkthrough

A single conditional check in the cursor highlight handler was modified to require entry.ctag in addition to the existing regex condition when handling entries with missing line numbers. This prevents regex-based match operations from executing when the required tag data is absent.

Changes

Cohort / File(s) Summary
Cursor highlight guard condition
lua/fzf-lua/previewer/builtin.lua
Added entry.ctag requirement to the conditional branch that clears/searches matches when lnum is absent, preventing operations without valid tag data.

Possibly related PRs

  • PR #2668: Modifies the same Previewer.buffer_or_file:set_cursor_hl(entry) function to add additional guards (entry.col > 0) for preventing invalid highlights; both PRs harden the highlight logic with complementary safety checks.

Poem

🐰 A little guard to keep things tight,
No ctag? No search, no highlight!
One line changed, the cursor's freed,
From phantom marks we don't quite need. ✨

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the fix: preventing ripgrep error messages from being incorrectly parsed as ctags patterns, which aligns with the actual code change.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch always-clear-match

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
lua/fzf-lua/previewer/builtin.lua (1)

1268-1283: Narrow fix works, but stale matchadd highlights can still linger across previews.

Adding entry.ctag to the guard correctly prevents rg error text from being treated as a ctag pattern, and matches the convention used in actions.lua (context snippet 1). However, fn.clearmatches() now lives inside this branch — so when a subsequent entry has no ctag/lnum (e.g., an rg error entry, or an entry that falls through to the lnum < 1 path at line 1285), any previous matchadd highlight applied during an earlier ctag-based preview in the same preview window will persist. Given the PR's "always-clear-match" framing, you may want to hoist the clear out of the conditional so it runs on every set_cursor_hl invocation.

♻️ Optional refinement
   local cached_pos = self.bcache:get_pos(buf)
   if type(cached_pos) ~= "table" then cached_pos = nil end
   local lnum, col = entry.line, math.max(1, entry.col or 1)
 
+  pcall(api.nvim_win_call, win, function() fn.clearmatches() end)
+
   if (not lnum or lnum == 0) and entry.ctag and regex then
-    -- pcall(fn.clearmatches, self.win.preview_winid)
     pcall(api.nvim_win_call, win, function()
       -- start searching at line 1 in case we
       -- didn't reload the buffer (same file)
       api.nvim_win_set_cursor(0, { 1, 0 })
-      fn.clearmatches()
       -- test the regex so we can alert the user of the search fail
       if not utils.vim_regex(regex, { silent = true }) then return end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lua/fzf-lua/previewer/builtin.lua` around lines 1268 - 1283, The
highlight-clearing call fn.clearmatches() is currently only run inside the
ctag/lnum branch, allowing previous matchadd highlights to persist for later
previews; move the clearmatches call so it always runs at the start of the
preview-window callback. Specifically, invoke pcall(api.nvim_win_call, win,
function() fn.clearmatches() ... end) (or place fn.clearmatches() at the top of
the existing api.nvim_win_call callback) before any conditional logic that
checks entry.ctag/lnum/regex, remove the duplicate inside that branch if
present, and keep the existing matchadd(fn.matchadd or hls.search) behavior only
when entry.ctag and regex are valid so highlights are applied but always cleared
first.
🤖 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/previewer/builtin.lua`:
- Around line 1268-1283: The highlight-clearing call fn.clearmatches() is
currently only run inside the ctag/lnum branch, allowing previous matchadd
highlights to persist for later previews; move the clearmatches call so it
always runs at the start of the preview-window callback. Specifically, invoke
pcall(api.nvim_win_call, win, function() fn.clearmatches() ... end) (or place
fn.clearmatches() at the top of the existing api.nvim_win_call callback) before
any conditional logic that checks entry.ctag/lnum/regex, remove the duplicate
inside that branch if present, and keep the existing matchadd(fn.matchadd or
hls.search) behavior only when entry.ctag and regex are valid so highlights are
applied but always cleared first.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d51632cb-f4ef-4baa-a432-e196ee714540

📥 Commits

Reviewing files that changed from the base of the PR and between 8342463 and 65b7b6b.

📒 Files selected for processing (1)
  • lua/fzf-lua/previewer/builtin.lua

@phanen
Copy link
Copy Markdown
Collaborator Author

phanen commented Apr 20, 2026

matchadd will remain even buf changed in window, so

Maybe this can also happened in FzfLua global.

If it happened, maybe if anyone mix ctags with lsp in FzfLua global...
(I'm not sure if this possible)
We need to move win_call clearmaches() out from branch and always run it.

@phanen phanen merged commit c27c398 into main Apr 20, 2026
11 checks passed
@phanen phanen deleted the always-clear-match branch April 20, 2026 09:50
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