plugins/rainbow: init #3574
plugins/rainbow: init #3574
Conversation
|
When you create such a PR, can you include a link to the plugin's homepage?
I don't think that this is a reason for us to enable it by default. Also, can you add |
saygo-png
force-pushed
the
plugins-rainbow-init
branch
from
b4239ad to
084c135
Compare
Sure thing.
Removed it.
Added an extensive example taken mostly from upstream, i only changed one of the colors to a hex notation to show that you can also define colors that way. |
| maintainers = [ lib.maintainers.saygo-png ]; | ||
| globalPrefix = "rainbow_"; | ||
|
|
||
| settingsExample = { |
There was a problem hiding this comment.
You can leave the example test case as is, but settingsExample here is used to generate the documentation. Hence, it would be better to shorten it to 5-10 options, otherwise it becomes quite unreadable.
There was a problem hiding this comment.
The defaults set by the plugin are almost identical to the example i took from the readme. So the default and example test cases almost test the same thing. Should i remove the example test case because of that?
There was a problem hiding this comment.
Preferred to have settingsExample and example be more real world configuration examples.
There was a problem hiding this comment.
Preferred to have
settingsExampleandexamplebe more real world configuration examples.
I can't really come up with anything more practical here. The issue is that the configuration for this plugin is mostly just colors, the rest is defined by default and works well. Majority of the fancy options are for fixing compatibility with other plugins but I've never had to use them.
Current example does show how you can disable filetypes, basic regex escaping from nix, and configure the colors which id wager is what most people will ever do with this plugin's configuration.
| empty = { | ||
| plugins.rainbow.enable = true; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Could you please add a defaults test case that explicitly sets each option to its default value, as documented upstream?
There was a problem hiding this comment.
I've added what i could, but sadly i just don't have the regex skills to properly escape these complex expressions in nix, in such a way that they also get escaped in lua while preserving their original meaning.
You can see what is missing from the defaults and the regex horror here: https://github.com/luochen1990/rainbow/blob/76ca1a20aa42edb5c65c19029968aad4625790dc/autoload/rainbow_main.vim
saygo-png
force-pushed
the
plugins-rainbow-init
branch
from
084c135 to
55d1f98
Compare
Signed-off-by: saygo-png <[email protected]>
saygo-png
force-pushed
the
plugins-rainbow-init
branch
from
55d1f98 to
533dbbf
Compare
https://github.com/luochen1990/rainbow/
Something easier to review from me this time.
This plugin adds rainbow parentheses. I'm aware that we already have rainbow-delimiters, but that uses treesitter. Not everyone likes that project. Even though I do use treesitter I find that this non TS implementation works much faster and does the same thing.
I enable the active setting by default because that is described as part of "setup" in the upstream documentation.