Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork354
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 ourterms of service andprivacy 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?
Uh oh!
There was an error while loading.Please reload this page.
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 uselib.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 theconfig arg:
| helpers.neovim-plugin.mkNeovimPluginconfig{ | |
| 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 intests/test-sources/plugins and/or recently merged PRs.
Generally, we like to have anempty test case, adefaults test case and (ideally) anexample 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: becausesettings is a freeform option, there is no need to declare sub-options forevery 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=mkListOflib.stypes.str[]"A list of filetypes to exclude from highlighting."; | |
| exclude_buftypes=mkListOflib.stypes.str[]"A list of buftypes to exclude from highlighting."; | |
| exclude_filetypes=mkListOflib.types.str[]"A list of filetypes to exclude from highlighting."; | |
| exclude_buftypes=mkListOflib.types.str[]"A list of buftypes to exclude from highlighting."; |
No description provided.