[vim/vim] Add checkgroups Makefile target to check default hl group links. (PR #16678)
cvwillegen
When adding highlight groups, you may need to add a "default link NewHlGroup ExistingHlGroup" in highlight.c code, so that when resetting a color scheme an old color will not be left behind.
Added a Makefile target in the 'runtime/doc' directory that checks if hl- groups that were added to the documentation are either reflected in the source code, or belong to a list of 'known to be ignored' highlight groups.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/16678
Commit Summary
- 9bbd445 Add checkgroups Makefile target to check default hl group links.
File Changes
(2 files)
- M runtime/doc/Makefile (16)
- A runtime/doc/hlgroups.ignore (68)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
cvwillegen
@cvwillegen pushed 1 commit.
- 72173ed Add 'known good' hl groups file to Filelist.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
cvwillegen
@cvwillegen pushed 1 commit.
- 9f1450d Check if removing "default link" breaks the build.
You are receiving this because you are subscribed to this thread.
cvwillegen
@cvwillegen pushed 2 commits.
You are receiving this because you are subscribed to this thread.
Christ van Willegen
@cvwillegen pushed 1 commit.
- 9f1450d Check if removing "default link" breaks the build.
Christian Brabandt
@chrisbra commented on this pull request.
In ci/hlgroups.make:
> @@ -0,0 +1,18 @@ +# vim: ft=make +SHELL = /bin/bash + +# Default target to actually run the comparison: +.PHONY: check +.INTERMEDIATE: hlgroups deflinks hlgroups.stripped + +check: hlgroups.stripped deflinks + diff hlgroups.stripped deflinks + +hlgroups: + grep '\*hl-' ../runtime/doc/*txt | sed -E 's/.*:<?\s*//' | sed 's/ /\n/g' | sed 's/hl-//' | sed 's/\*//g' | sort > hlgroups
This seems a bit ugly. Perhaps use a separate sed script like this:
#!/bin/sh
sed -ne '/\*hl-/{
s/^<\?\s*//
s/\*//g
s/hl-//g
s/ /\n/
p
}' ../runtime/doc/*.txt |sort
or put all the sed invocations into a single command using several -e args.
In src/highlight.c:
> @@ -261,7 +261,6 @@ static char *(highlight_init_both[]) = {
"default link PmenuMatch Pmenu",
"default link PmenuMatchSel PmenuSel",
"default link PmenuExtra Pmenu",
- "default link PmenuExtraSel PmenuSel",
I don't understand that. Why do we need to remove the default link for PmenuExtraSel?
In ci/hlgroups.make:
> @@ -0,0 +1,18 @@ +# vim: ft=make +SHELL = /bin/bash + +# Default target to actually run the comparison: +.PHONY: check +.INTERMEDIATE: hlgroups deflinks hlgroups.stripped + +check: hlgroups.stripped deflinks + diff hlgroups.stripped deflinks + +hlgroups: + grep '\*hl-' ../runtime/doc/*txt | sed -E 's/.*:<?\s*//' | sed 's/ /\n/g' | sed 's/hl-//' | sed 's/\*//g' | sort > hlgroups
Do we need a clean rule?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#16678)shouldn't we call this script in https://github.com/vim/vim/blob/master/.github/workflows/ci.yml like the vimtags target?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christ van Willegen
shouldn't we call this script in https://github.com/vim/vim/blob/master/.github/workflows/ci.yml like the vimtags target?
vim-dev ML
vim-ml left a comment (vim/vim#16678)—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christ van Willegen
Yes, I'm trying to get this in CI.
vim-dev ML
vim-ml left a comment (vim/vim#16678)—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
zeertzjq
Reopened #16678.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
cvwillegen
@cvwillegen pushed 2 commits.
You are receiving this because you are subscribed to this thread.
cvwillegen
@cvwillegen pushed 1 commit.
- a62e6f3 Merge branch 'master' into check_highlight_groups
You are receiving this because you are subscribed to this thread.
cvwillegen
@cvwillegen pushed 1 commit.
- f00c6a8 Move checking hlgroups down in the ci workflow.
You are receiving this because you are subscribed to this thread.
cvwillegen
@cvwillegen pushed 1 commit.
- 5e2ed81 Run the Makefile from the correct directory.
You are receiving this because you are subscribed to this thread.
cvwillegen
@cvwillegen pushed 1 commit.
- 507d8ba Re-add intentionally left out highlight group, stop ignoring handeled highlight groups.
You are receiving this because you are subscribed to this thread.
cvwillegen
@cvwillegen commented on this pull request.
In ci/hlgroups.make:
> @@ -0,0 +1,18 @@ +# vim: ft=make +SHELL = /bin/bash + +# Default target to actually run the comparison: +.PHONY: check +.INTERMEDIATE: hlgroups deflinks hlgroups.stripped + +check: hlgroups.stripped deflinks + diff hlgroups.stripped deflinks + +hlgroups: + grep '\*hl-' ../runtime/doc/*txt | sed -E 's/.*:<?\s*//' | sed 's/ /\n/g' | sed 's/hl-//' | sed 's/\*//g' | sort > hlgroups
or put all the sed invocations into a single command using several
-eargs.
I ended up doing exactly that, thanks for the hint!
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
cvwillegen
@cvwillegen commented on this pull request.
In ci/hlgroups.make:
> @@ -0,0 +1,18 @@ +# vim: ft=make +SHELL = /bin/bash + +# Default target to actually run the comparison: +.PHONY: check +.INTERMEDIATE: hlgroups deflinks hlgroups.stripped + +check: hlgroups.stripped deflinks + diff hlgroups.stripped deflinks + +hlgroups: + grep '\*hl-' ../runtime/doc/*txt | sed -E 's/.*:<?\s*//' | sed 's/ /\n/g' | sed 's/hl-//' | sed 's/\*//g' | sort > hlgroups
Do we need a clean rule?
I added the intermediate files in the .INTERMEDIATE Makefile target, so a clean rule is not needed any more.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
cvwillegen
cvwillegen left a comment (vim/vim#16678)I think this is done now.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you commented.
Christian Brabandt
chrisbra left a comment (vim/vim#16678)thanks
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Christian Brabandt
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.