-
-
Notifications
You must be signed in to change notification settings - Fork 12
feat(foldtext): allow function() for virtual text components #25
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
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.
Other than the issues with specific parts of the PR, I am not really sure what problem this PR solves.
What is the use case for being able to dynamically show more info for your fold?
I use a buffer-local variable to depict the 1 active buffer i'm working in. Decorations in all other buffers are muted to prevent death-by-decorative-signs because otherwise all open buffers show a wide range of decorations that require attention, simultaneously. See attached screendumps here below. Not really sure what you mean with the type annotation BTW. If you're not in favour of this functionality, no worries, just let me know so I can purge the PR and keep a local fork. |
I see, yeah that makes sense. By type annotation I mean the ---@ comments for the lua-lsp. If you are not familiar with those, I can also add them, just be sure to address the rest of the issues then. |
0d99b07 to
32e521d
Compare
b49ffec to
f25b61c
Compare
3217792 to
3700170
Compare
3700170 to
84604a5
Compare
|
I think i've resolved all your comments. I've tested the changes locally, LGTM. |
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 PR is fine, it's just a bunch of small consistency/formatting/typing issues.
Other than the things I pointed in the code:
- run stylua on the end result for consistent formatting
- use camelCase for variable names, as per PR checklist
- update the readme as well (i.e., update the default config with your type annotations)
Don't mean to be nitpicky, consistent formatting and correct typing is just important to keep a code base clean when it grows larger. (Generally, for PRs to nvim plugins, I recommend using the lua LSP and running stylua with the repo-specific settings, since a lot of plugins have tests for this, not just mine.)
| { lineCountText, { config.foldtext.lineCount.hlgroup } }, | ||
| } | ||
| if config.foldtext.diagnosticsCount then | ||
| local show_diagnostics |
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 whole block can be simplified as:
local showDiagnostics = config.foldtext.diagnosticsCount
if type(showDiagnostics) == "function" then showDiagnostics = showDiagnostics() end| vim.list_extend(virtText, diagnostics) | ||
| end | ||
| if config.foldtext.gitsignsCount then | ||
| local show_gitsigns |
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 same simplification as mentioned above can be done here
| }, | ||
| ---@type boolean | fun():boolean | ||
| diagnosticsCount = true, -- uses hlgroups and icons from `vim.diagnostic.config().signs` | ||
| ---@type boolean | fun():boolean |
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 function signature does not match the type used (3 parameters), which is why the nvim type check fails.
| if config.foldtext.diagnosticsCount then | ||
| local show_diagnostics | ||
| if type(config.foldtext.diagnosticsCount) == "function" then | ||
| show_diagnostics = config.foldtext.diagnosticsCount() |
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.
Why does the gitsigns function expect 3 parameters, but this one does not expect one? Wouldn't it make more sense to have both of them expect the same input?
| enabled = true, | ||
| padding = 3, | ||
| lineCount = { | ||
| ---@type string | fun():string |
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 function signature does not match the type used (3 parameters), which is why the nvim type check fails.


What problem does this PR solve?
Virtual text components can only be directed using booleans / strings
How does the PR solve it?
Functions can be used to toggle or change strings for virtual text components
Checklist
camelCasevariable names.README.md(the.txtfile is auto-generated and does not need to bemodified).