Skip to content

fix(previewer): don't set hl on invalid col#2668

Merged
phanen merged 1 commit intomainfrom
invalid-col
Apr 15, 2026
Merged

fix(previewer): don't set hl on invalid col#2668
phanen merged 1 commit intomainfrom
invalid-col

Conversation

@phanen
Copy link
Copy Markdown
Collaborator

@phanen phanen commented Apr 15, 2026

regression of 7162adf

:FzfLua buffers can notice the difference

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed cursor highlighting in file previews to prevent invalid highlight positions.

regression of 7162adf

`:FzfLua buffers` can notice the difference
@phanen
Copy link
Copy Markdown
Collaborator Author

phanen commented Apr 15, 2026

Ok now I know why it's written like that...

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 15, 2026

📝 Walkthrough

Walkthrough

Updated a cursor-highlight extmark fallback condition in the previewer builtin module to only create extmarks when the column value is explicitly greater than zero, preventing invalid non-positive column values from being used.

Changes

Cohort / File(s) Summary
Cursor Highlight Condition
lua/fzf-lua/previewer/builtin.lua
Changed extmark fallback condition from truthy check to explicit entry.col > 0 to prevent creation of extmarks with invalid column values.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 A column must be true and greater still,
Not zero, not negative, but positive with skill,
One tiny fix, so precise and right,
The cursor highlight now shines bright! ✨

🚥 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 concisely describes the main change: preventing highlight from being set on invalid column values in the previewer.
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 invalid-col

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.

@phanen
Copy link
Copy Markdown
Collaborator Author

phanen commented Apr 15, 2026

I don't know why I didn't notice this for months...
Also it's amazing bufinfo can provide lnum but not col..
I used to believe nvim cursor is only tied to window

@phanen phanen merged commit 6ce094a into main Apr 15, 2026
10 of 11 checks passed
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.

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/previewer/builtin.lua`:
- Line 1332: The condition "if not extmark and hls.cursor and entry.col > 0
then" can error when entry.col is nil; change it to explicitly guard entry.col
(e.g., check entry.col ~= nil and entry.col > 0, or use (entry.col or 0) > 0) so
the branch only evaluates numeric comparisons when col exists; keep references
to extmark and hls.cursor unchanged and ensure this nil-safe check covers
entries produced by the autocmd parser that may omit the col field.
🪄 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: 76f2b9a7-e132-4dd8-ad71-2c178a5c9868

📥 Commits

Reviewing files that changed from the base of the PR and between b9f947a and 050fc88.

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

Comment thread lua/fzf-lua/previewer/builtin.lua
@phanen
Copy link
Copy Markdown
Collaborator Author

phanen commented Apr 15, 2026

https://github.com/neovim/neovim/blob/a940b77cb25ae12cc1269cf64bdede698a4c5aac/src/nvim/eval/buffer.c#L561-L562

it's still w_cursor.. but buffer will have a b_wininfo list, so if buf is "hide", each window that show it before will "remeber" its last postion.

Not sure why it did't provide lnum but not col... maybe similar to why we only have :edit +lnum but not col:

							*+cmd* *[+cmd]*
The [+cmd] argument can be used to position the cursor in the newly opened
file, or execute any other command:
	+		Start at the last line.
	+{num}		Start at line {num}.

@phanen phanen deleted the invalid-col branch April 16, 2026 04:49
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