Add support for vim-clap

[meck]

This adds highlights for vim-clap

[arcticicestudio]

Hi @meck 👋, thanks for your contribution 👍
I'll review it in the near future and get back to you.

[Arelav]

Can this be merged, please?

[sdemura]

I would love to see this merged!

[ikalnytskyi]

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?

Suggested change

	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

[meck]

LGTM @arcticicestudio can decide

[arcticicestudio]

Sorry for this delay, life, other projects and tons of Nord issues/PRs keeps me busy 😐
I haven't checked and tested the actual code, but I'd like to throw in some thoughts regarding the design.
Nord's main accent is nord8 that can be combined with the other colors of the Frost palette. While Aurora colors like nord12 or nord13 might be used more often in other themes, Nord tries to keep they purpose as signal colors to style e.g. warnings and errors.
Therefore I'd like to propose to change the style for Vim Clap to use

• nord4 as default foreground color.
• nord8 as main color for matches
• nord9 and nord10 for sub-matches since they have a darker appearance and nord8 should get the user focus while these both indicate that it might also match.

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
This is what is looks like when using the design I've described above:

Thanks again for the active feedback on this PR, really like the way the community works on such things 😄
Let me know what you think if we'd use this design for Vim Clap too.

[meck]

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.

[liuchengxu]

Then there is the question if there should be a theme file in here and in the clap repo as well?

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 let g:clap_theme = 'nord' explicitly. So I would second the former way if the clap theme has a original vim colorscheme.

[arcticicestudio]

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.
Some examples where incompatibles have been the root cause are #187 and #193 where the users installed a plugin called flazz/vim-colorschemes that includes themes for vim-airline/vim-airline theme, among other theme for Nord.

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.
This way only Nord as color scheme affects the actual highlighting that is managed by Vim while vim-clap doesn't need to care about the color scheme at all.
Adding highlighting rules in both now adds a two-direction dependency because now vim-clap actually cares about the highlighting, but might be affected when the user also sets Nord as active color scheme that includes settings for vim-clap specific highlighting groups. Updating one of them might result in different styles when used with or without Nord as active color scheme.

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.
But since the PR in the vim-clap repository has already been merged, I guess there's no more reason to decide which solution might fit 😄

Anyway, I hope to find some free time again to work on Nord so I can finally test this PR and merge it.

[arcticicestudio]

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.

[arcticicestudio]

---

Release Notes Assets