-
-
Notifications
You must be signed in to change notification settings - Fork 352
plugins/nvim-highlight-colors: init #2105
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?
plugins/nvim-highlight-colors: init #2105
Conversation
|
This adds a plugin for nvim-highlight-colors, it is a draft as the corresponding vimPlugin does not exist in nixpkgs and I do not intend to contribute it upstream. |
In that case, is that even possible to add such a plugin to |
|
Also, I see that your plugin is indeed in nixpkgs: https://search.nixos.org/packages?channel=24.05&show=vimPlugins.nvim-highlight-colors&from=0&size=50&sort=relevance&type=packages&query=nvim-highlight-colors. |
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 CI failure seems to just be a typo. Otherwise this is more or less there, suggestions below are fairly minor. 😁
| @@ -0,0 +1,96 @@ | |||
| { | |||
| lib, | |||
| helpers, | |||
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.
helpers is deprecated, you can use lib.nixvim instead:
| helpers, |
| inherit (helpers.defaultNullOpts) | ||
| mkBool | ||
| mkEnum | ||
| mkListOf | ||
| mkStr | ||
| mkStr' | ||
| ; |
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.
To match the style used elsewhere in nixvim:
| inherit (helpers.defaultNullOpts) | |
| mkBool | |
| mkEnum | |
| mkListOf | |
| mkStr | |
| mkStr' | |
| ; | |
| inherit (lib.nixvim) defaultNullOpts; |
| mkStr' | ||
| ; | ||
| in | ||
| helpers.neovim-plugin.mkNeovimPlugin config { |
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.
We (very) recently dropped the config arg:
| helpers.neovim-plugin.mkNeovimPlugin config { | |
| lib.nixvim.neovim-plugin.mkNeovimPlugin { |
| in | ||
| helpers.neovim-plugin.mkNeovimPlugin config { | ||
| name = "nvim-highlight-colors"; | ||
| defaultPackage = pkgs.vimPlugins.nvim-highlight-colors; |
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.
Watch out for #2139, if it is merged first you'll have to do:
| defaultPackage = pkgs.vimPlugins.nvim-highlight-colors; | |
| package = "nvim-highlight-colors"; |
| name = "nvim-highlight-colors"; | ||
| defaultPackage = pkgs.vimPlugins.nvim-highlight-colors; | ||
|
|
||
| maintainers = [ helpers.maintainers.thubrecht ]; |
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.
| maintainers = [ helpers.maintainers.thubrecht ]; | |
| maintainers = [ lib.maintainers.thubrecht ]; |
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.
Could you introduce a test file please, you can reference other tests in tests/test-sources/plugins and/or recently merged PRs.
Generally, we like to have an empty test case, a defaults test case and (ideally) an example test case.
|
|
||
| maintainers = [ helpers.maintainers.thubrecht ]; | ||
|
|
||
| settingsOptions = { |
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.
Note: because settings is a freeform option, there is no need to declare sub-options for every upstream plugin option. Users can define any config they like, so having too many options can just end up being a maintenance burden.
The judgement call is yours to make though, I won't block a PR for having "too many" settings options 😁
| render = mkEnum [ | ||
| "background" | ||
| "foreground" | ||
| "virtual" | ||
| ] "background" "The render style used."; |
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.
| render = mkEnum [ | |
| "background" | |
| "foreground" | |
| "virtual" | |
| ] "background" "The render style used."; | |
| render = defaultNullOpts.mkEnumFirstDefault [ | |
| "background" | |
| "foreground" | |
| "virtual" | |
| ] "The render style used."; |
| exclude_filetypes = mkListOf lib.stypes.str [ ] "A list of filetypes to exclude from highlighting."; | ||
| exclude_buftypes = mkListOf lib.stypes.str [ ] "A list of buftypes to exclude from highlighting."; |
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.
Typo:
| exclude_filetypes = mkListOf lib.stypes.str [ ] "A list of filetypes to exclude from highlighting."; | |
| exclude_buftypes = mkListOf lib.stypes.str [ ] "A list of buftypes to exclude from highlighting."; | |
| exclude_filetypes = mkListOf lib.types.str [ ] "A list of filetypes to exclude from highlighting."; | |
| exclude_buftypes = mkListOf lib.types.str [ ] "A list of buftypes to exclude from highlighting."; |
No description provided.