-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add virtual text counter display #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add new counter.lua module to display [X/Y] reference count - Show counter as virtual text at end of line when jumping - Counter clears automatically when highlights are cleared - Configurable via counter.enable and counter.hl_group options - Default styling: bold orange text for high visibility
mawkler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
I was also thinking that perhaps you should break out the logic for getting the current index and the total number of references to its own function. That way one could use it if they for example wanted to instead display that information in the statusline instead of using virtual text. That function would return the two values current_reference_index, total_reference_count (or with similar names). I notice however when looking through my code that we're passing around state a lot, including the references and current reference. So perhaps the best way would be to create a Lua "object" for storing that state. What do you think?
- Add lua/refjump/state.lua for per-buffer state storage - Expose get_reference_info() for statusline integration - Remove counter.hl_group option, link RefjumpCounter to WarningMsg - Refactor find_reference_index() to use vim.iter - Move counter.enable check to jump.lua with show_counter() helper - Add test infrastructure with plenary.nvim (12 tests) - Update CI workflow to run tests on push/PR
The state was being cleared immediately on CursorMoved after a jump, preventing lualine/statusline from reading reference info. - Restore highlight_references toggle flag in highlight.lua - Add highlight.is_active() to expose the flag - Update state.is_active() to delegate to highlight.is_active() - Fix disable() to use toggle pattern: first call flips flag, second clears - Update tests to reflect new is_active() behavior
- Document counter.enable option in configuration - Document RefjumpCounter highlight group - Add statusline integration section with lualine example
|
Hey @mawkler, thanks for the feedback! I've addressed all your comments:
|
mawkler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very good! I have a couple of smaller thoughts on your changes though.
| ---Find the index of a reference in the references list | ||
| ---@param reference RefjumpReference | ||
| ---@param references RefjumpReference[] | ||
| ---@return integer|nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very minor nit, but everywhere where you have type | nil you can replace with type? which is slightly cleaner in my opinion
| if not highlight_references then | ||
| vim.api.nvim_buf_clear_namespace(0, highlight_namespace, 0, -1) | ||
| require('refjump.counter').clear(0) | ||
| require('refjump.state').clear() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The state shouldn't be cleared here, right? If the user just wants to disables the highlights, they might still want to repeat the same reference jump with demicolon.nvim, and so in that case it's still useful to reuse the state
You'll then have to change your statusline example in the README to use the is_active() method to check if the component should be displayed or not
| ---Get info about current reference position (for statusline use) | ||
| ---@param bufnr? integer Buffer number (defaults to current buffer) | ||
| ---@return { index: integer|nil, total: integer } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to repeat this documentation, it gets reused from the assigned require('refjump.state').get_reference_info, so you can just remove it here
| ---Check if reference navigation is currently active | ||
| ---@return boolean | ||
| function M.is_active() | ||
| return require('refjump.highlight').is_active() | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this method should be moved to lua/refjump/init.lua instead since this is more of a user facing method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding tests! If you don't mind, could you add a couple of lines to the README that explains how to run the tests?
| name: Pandoc to Vimdoc | ||
| runs-on: ubuntu-latest | ||
| needs: test | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this conditional neccessary? Isn't this already covered by lines 3-4 in this file?
Summary
[3/12])counteroptionsChanges
lua/refjump/counter.lua- handles virtual text displaylua/refjump/jump.lua- calls counter instead of vim.notifylua/refjump/highlight.lua- clears counter with highlightslua/refjump/init.lua- adds counter configuration optionsConfiguration
Demo
When jumping between references with
]r/[r, the counter appears at the end of the line:[1/5],[2/5], etc.Replaces the previous vim.notify() notification with inline display inspired by nvim-hlslens.