-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
Add support for vim-clap #178
Conversation
Hi @meck 👋, thanks for your contribution 👍 |
Can this be merged, please? |
I would love to see this merged! |
colors/nord.vim
Outdated
let clap_matches = [ | ||
\ [s:nord11_gui, s:nord11_term] , | ||
\ [s:nord13_gui, s:nord13_term] , | ||
\ [s:nord14_gui, s:nord14_term] , | ||
\ [s:nord11_gui, s:nord11_term] , | ||
\ [s:nord13_gui, s:nord13_term] , | ||
\ [s:nord14_gui, s:nord14_term] , | ||
\ [s:nord11_gui, s:nord11_term] , | ||
\ [s:nord13_gui, s:nord13_term] , | ||
\ ] | ||
let idx = 1 | ||
for g in clap_matches | ||
call s:hi("ClapMatches" . idx, g[0], "", g[1], "", "", "") | ||
let idx += 1 | ||
endfor | ||
let clap_fuzzy_matches = [ | ||
\ [s:nord11_gui, s:nord11_term] , | ||
\ [s:nord13_gui, s:nord13_term] , | ||
\ [s:nord14_gui, s:nord14_term] , | ||
\ [s:nord11_gui, s:nord11_term] , | ||
\ [s:nord13_gui, s:nord13_term] , | ||
\ [s:nord14_gui, s:nord14_term] , | ||
\ [s:nord11_gui, s:nord11_term] , | ||
\ [s:nord13_gui, s:nord13_term] , | ||
\ [s:nord14_gui, s:nord14_term] , | ||
\ [s:nord11_gui, s:nord11_term] , | ||
\ [s:nord13_gui, s:nord13_term] , | ||
\ [s:nord14_gui, s:nord14_term] , | ||
\ ] | ||
let idx = 1 | ||
for g in clap_fuzzy_matches | ||
call s:hi("ClapFuzzyMatches" . idx, g[0], "", g[1], "", "", "") | ||
let idx += 1 | ||
endfor |
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.
Good work! I'm not maintainer of this repo, but may I suggest a slight improvement over this code?
It seems clap_matches
and clap_fuzzy_matches
are pretty much the same (the only difference is length). In fact, they are only 3 colors that we want repeat over and over again. So my suggestion is simply maintain a table of 3 colors, and pick the right one based on the index of ClapMatch
we're creating.
The only side effect is that we will produce more highlight rules for ClapMatch
than the plugin supports (12 instead of 8), but in my opinion it's not a big deal. What do you think?
let clap_matches = [ | |
\ [s:nord11_gui, s:nord11_term] , | |
\ [s:nord13_gui, s:nord13_term] , | |
\ [s:nord14_gui, s:nord14_term] , | |
\ [s:nord11_gui, s:nord11_term] , | |
\ [s:nord13_gui, s:nord13_term] , | |
\ [s:nord14_gui, s:nord14_term] , | |
\ [s:nord11_gui, s:nord11_term] , | |
\ [s:nord13_gui, s:nord13_term] , | |
\ ] | |
let idx = 1 | |
for g in clap_matches | |
call s:hi("ClapMatches" . idx, g[0], "", g[1], "", "", "") | |
let idx += 1 | |
endfor | |
let clap_fuzzy_matches = [ | |
\ [s:nord11_gui, s:nord11_term] , | |
\ [s:nord13_gui, s:nord13_term] , | |
\ [s:nord14_gui, s:nord14_term] , | |
\ [s:nord11_gui, s:nord11_term] , | |
\ [s:nord13_gui, s:nord13_term] , | |
\ [s:nord14_gui, s:nord14_term] , | |
\ [s:nord11_gui, s:nord11_term] , | |
\ [s:nord13_gui, s:nord13_term] , | |
\ [s:nord14_gui, s:nord14_term] , | |
\ [s:nord11_gui, s:nord11_term] , | |
\ [s:nord13_gui, s:nord13_term] , | |
\ [s:nord14_gui, s:nord14_term] , | |
\ ] | |
let idx = 1 | |
for g in clap_fuzzy_matches | |
call s:hi("ClapFuzzyMatches" . idx, g[0], "", g[1], "", "", "") | |
let idx += 1 | |
endfor | |
let clap_matches = [ | |
\ [s:nord11_gui, s:nord11_term] , | |
\ [s:nord13_gui, s:nord13_term] , | |
\ [s:nord14_gui, s:nord14_term] , | |
\ ] | |
let idx = 1 | |
while idx <= 12 | |
let clap_match_color = clap_matches[idx % len(clap_matches) - 1] | |
call s:hi("ClapMatches" . idx, clap_match_color[0], "", clap_match_color[1], "", "", "") | |
call s:hi("ClapFuzzyMatches" . idx, clap_match_color[0], "", clap_match_color[1], "", "", "") | |
let idx += 1 | |
endwhile |
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.
LGTM @arcticicestudio can decide
Sorry for this delay, life, other projects and tons of Nord issues/PRs keeps me busy 😐
I use fzf as my main fuzzy finder for almost everything so I also use it together with fzf.vim. While there's no official Nord port project for it (yet) I created a Nord theme for my personal use that is embedded into my ZSH .dotfiles Thanks again for the active feedback on this PR, really like the way the community works on such things 😄 |
I've cleaned this up a bit and conformed it to @arcticicestudio's suggestions, however I noticed Clap now has a auto-loaded theme system as used in liuchengxu/vim-clap#420. So it should probably be moved there (The same as airline and sightline)? Also all the providers have their own hl-groups, I've added a couple but the rest should probably be added as well. Then there is the question if there should be a theme file in here and in the clap repo as well? I guess that is something for @liuchengxu and @arcticicestudio to agree on. |
The advantage of putting it in the colorscheme repo is when people use this colorscheme, they'll have the clap support aumatically and don't have to add the line |
This is always a question that depends on the targeted application. Having builtin theme support in plugins is comfortable for users because in most cases they only need to specify the theme name in their configuration, but this comes with the price of maybe outdated themes and possible incompatibles. Contrary to my own statement in the pull request that merged Nord's airline theme into the origin repository, this indeed adds maintenance overhead. For example #128 updated the airline theme included in this repository to be more compatible with the edkolev/tmuxline.vim plugin, but this means the theme in the airline repository is now outdated. Normally you would now have to assume that the maintainers of the airline plugin are monitoring the repositories of the themes that are also included in their repository, but to be truth no one can expect that, least of all if everyone works on such projects in their free time. On the other side, adding support for other plugins in a color scheme is a better way to separate the actual dependencies between both and prevent conflicts: One-direction dependency from Nord → vim-clap. I don't know how theme loading has been implemented in vim-clap, but it should ensure to not override highlighting settings of the currently active Vim color scheme to also prevent incompatibility problems. Anyway, I hope to find some free time again to work on Nord so I can finally test this PR and merge it. |
I guess this was added because this PR is created from the default (develop) branch instead of a story branch so changed to the fork are reflected here. Removed it and so it can maybe be added back in a new PR when it was intended to be included. Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com>
Use `nord9` for the signs that mark a entry as selected. nordthemeGH-178 Co-authored-by: Sven Greb <development@svengreb.de>
Before the `s:i` variable would be scoped to the whole plugin which could lead to unexpected behaviour when it gets reused. nordthemeGH-178 Co-authored-by: Sven Greb <development@svengreb.de>
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.
Thanks again for your contribution 👍
I've added some small changes like removing the unrelated change to the LSP highlighting groups that was reflected to this PR because the main branch (develop
) was used instead of a story branch.
Also the s:i
variable in the loop was script-scoped which means it could lead to unexpected behavior when reused again later on. I've additionally reordered the groups so custom definitions appear before linked groups.
Added basic support for vim-clap [1], a modern and performant generic finder and dispatcher for Vim and NeoVim. [1]: https://github.com/liuchengxu/vim-clap GH-178 Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com> Co-authored-by: Sven Greb <development@svengreb.de>
* adds coc error gutter support * remove duplicated entries * Update colors/nord.vim Co-Authored-By: Arctic Ice Studio <development@arcticicestudio.com> * Fix typo line 13 * Uniform status lines config for bundled airline and lightline themes (nordtheme#169) The included theme bundles have not supported the "uniform status line" feature (nordthemeGH-58), which allows to change the background color of the status bar (StatusLine) group to `nord3`. Related to nordthemeGH-58 Resolves nordthemeGH-168 * Plugin support for `vim-startify` (nordtheme#176) Added custom highlight groups of the `vim-startify` (1) plugin to adapt to Nord's style. References: (1) https://github.com/mhinz/vim-startify Resolves nordthemeGH-159 * Remove underline from gutter line numbers (nordtheme#185) Vim version 8.1.2029 [1] added the `underline` attribute for the `CursorLineNr` group to `cterm` [2] based on vim/vim#4933 [3]. This change resulted in gutter line numbers being underlined which has now been reverted back to Nord's style by explicitly setting the attribute for the group to `NONE`. [1]: https://github.com/vim/vim/releases/tag/v8.1.2029 [2]: vim/vim@d9b0d83...017ba07#diff-80fffb3e9c20e93e5b2328a9a20e19c9 [3]: vim/vim#4933 Resolves nordthemeGH-174 * Prepare release version 0.13.0 * adds coc error gutter support * Add nvim-lsp support (nordtheme#198) Added highlighting support for the build-in Neovim language server [1] using the coc.nvim [2] groups as reference. [1]: https://github.com/neovim/nvim-lsp [2]: https://github.com/neoclide/coc.nvim * Consistent `Error` and MoreMsg highlight group consistent between console and GUI modes. (nordtheme#202) Consistent `Error` and `MoreMsg` highligh. in term and GUI mode (nordtheme#202) Before the `Error` group in GUI mode used `nord0` as foreground color instead of `nord4` resulting in a bad contrast. Also after checking ( links to it) Also since there was also no color defined for terminal mode for the `MoreMsg` group (see `:help MoreMsg` that link to `:help more-prompt`) Vim used the default color which was some kind of green. To ensure it matches Nord's style it has now been changed to use `nord8` (main accent color) for both terminal and GUI mode. This can be tested by running `:echon "MESSAGE\n"` taht produces a lot of lines that won't fit on the current screen space anymore. Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com> Co-authored-by: Sven Greb <development@svengreb.de> * Use transparent background for gutter line number in GUI mode (nordtheme#204) The `LineNr` and `CursorLineNr` highlight groups now have a transparent background in GUI mode. Before it was set to `nord0_gui` which worked fine in most cases. However, some plugins use these highlight groups to render their content in a popup window which can potentially have a different background color. This caused some issues e.g. for the fuzzy search plugin LeaderF [1]. The compatibility with the `g:nord_cursor_line_number_background` theme configuration has been verified to work as expected in both modes when it is set to `0` or `1`. This change is not related to the terminal mode or when using `set notermguicolors` since `ctermbg` for `LineNr` and `CursorLineNr` is set to `NONE` by default. [1]: https://github.com/Yggdroot/LeaderF * Release version 0.14.0 * adds coc error gutter support * Add nvim-lsp support (nordtheme#198) Added highlighting support for the build-in Neovim language server [1] using the coc.nvim [2] groups as reference. [1]: https://github.com/neovim/nvim-lsp [2]: https://github.com/neoclide/coc.nvim * Add basic TypeScript and improve TSX support (nordtheme#208) Basic highlighting support for TypeScript & TSX Added basic support to highlight TypeScript & TSX syntax more consistently through the HerringtonDarkholme/yats.vim plugin [1]. This includes improvements to highlight... 1. ...TypeScript interface an class names using `nord7` as foreground, where interfaces also use the bold attribute, to match with structs/classes. 2. ...global methods like e.g. `setTimeout` with `nord8` using the italic attribute to mark it kind of static. 3. ...regular expressions with `nord13` as foreground color instead of the normal color for quoted strings (`nord14`) to make it easier to differ between both. 4. ...global objects like `Error`, `JSON` and `console` with `nord7`. 5. ...primitive/builtin types like `string` with `nord9`. 6. ...TypeScript type references with `nord7`. 7. ...TypeScript specific characters like for type annotations (`:`) and member optionality (`?`) as operator with `nord9`. This also includes improvements for "vanilla" JavaScript elements. [1]: https://github.com/HerringtonDarkholme/yats.vim Resolves nordthemeGH-208 Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com> Co-authored-by: Sven Greb <development@svengreb.de> * Add support for vim-clap (nordtheme#178) Added basic support for vim-clap [1], a modern and performant generic finder and dispatcher for Vim and NeoVim. [1]: https://github.com/liuchengxu/vim-clap nordthemeGH-178 Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com> Co-authored-by: Sven Greb <development@svengreb.de> * Release version 0.15.0 Co-authored-by: Arctic Ice Studio <development@arcticicestudio.com> Co-authored-by: john.hennessey <john.hennessey@ef.com> Co-authored-by: John Hennessey <jghennes@gmail.com> Co-authored-by: Radu Vasilescu <vasilescur@gmail.com> Co-authored-by: Jose M. Murinello <jm.murinello@gmail.com> Co-authored-by: Alexander Jeurissen <1220084+alexanderjeurissen@users.noreply.github.com> Co-authored-by: xulongwu4 <xulongwu4@gmail.com> Co-authored-by: Sven Greb <development@svengreb.de> Co-authored-by: iamdi <helloiamdi@gmail.com> Co-authored-by: Johan <meck@users.noreply.github.com>
This adds highlights for vim-clap