[vim/vim] Remove diff* links added in #13776 and doc added in commit b1392be (PR #13825)

69 views
Skip to first unread message

Romain Lafourcade

unread,
Jan 6, 2024, 8:06:15 AM1/6/24
to vim/vim, Subscribed

The links added in #13776 are way too noisy for the contexts in which the diff syntax is applied (git commits, patches, etc.).

This commit…

  • removes those links
  • adds hopefully semantically correct styling for diffAdded, diffChanged, and diffRemoved, valid when using the default colorscheme and other colorschemes that don't explicitly override those highlight groups

For reference:

You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/13825

Commit Summary

  • b4e21fe Remove diff* links added in #13776 and doc added in commit b1392be

File Changes

(4 files)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825@github.com>

Christian Clason

unread,
Jan 6, 2024, 8:13:03 AM1/6/24
to vim/vim, Subscribed

This essentially adds new filetype-specific "default group names" (as in :h group-name); if this is an option, this is clearly a better alternative than relying on default links.

I think this is a good idea, since the current, programming language-oriented, default groups are a poor fit for many markup or data languages (Markdown, Vimdoc, LaTeX, JSON, ...)

Are you generally considering adding more of such groups (as need arises, and obviously not in this PR)?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1879683192@github.com>

Christian Brabandt

unread,
Jan 6, 2024, 8:39:41 AM1/6/24
to vim/vim, Subscribed

Thanks, definitely an improvement. Can you please also document those new groups at :h group-name?

Are you generally considering adding more of such groups (as need arises, and obviously not in this PR)?

If need arises

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1879690353@github.com>

Christian Clason

unread,
Jan 6, 2024, 8:46:17 AM1/6/24
to vim/vim, Subscribed

(And you probably want to update the bundled colorschemes to account for this change.)

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1879692151@github.com>

dkearns

unread,
Jan 6, 2024, 9:33:56 AM1/6/24
to vim/vim, Subscribed

This essentially adds new filetype-specific "default group names" (as in :h group-name);

Should these be capitalised?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1879714312@github.com>

Maxim Kim

unread,
Jan 6, 2024, 5:05:20 PM1/6/24
to vim/vim, Subscribed

In default gvim (white bg), diffAdded is kind of hard to read:

image.png (view on web)

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1879843042@github.com>

Maxim Kim

unread,
Jan 6, 2024, 5:14:42 PM1/6/24
to vim/vim, Subscribed

What about DarkGreen for light bg?

image.png (view on web)

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1879845540@github.com>

Romain Lafourcade

unread,
Jan 7, 2024, 3:47:01 AM1/7/24
to vim/vim, Push

@romainl pushed 2 commits.

  • 09caae5 Use darkgreen instead of green for "light" background
  • 8c9a63d Use a more subtle blue for GUI and improve documentation

—
View it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/push/16534631484@github.com>

Romain Lafourcade

unread,
Jan 7, 2024, 3:59:43 AM1/7/24
to vim/vim, Push

@romainl pushed 1 commit.

  • 7caf667 Use limegreen instead of green and darkgreen for diffAdded in GUI

—
View it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/push/16534688966@github.com>

Maxim Kim

unread,
Jan 7, 2024, 4:05:29 AM1/7/24
to vim/vim, Subscribed

limegreen is still too bright to my taste/eyes

image.png (view on web)

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1879999029@github.com>

Romain Lafourcade

unread,
Jan 7, 2024, 4:08:03 AM1/7/24
to vim/vim, Subscribed

I've…

  • added the three names to :help group-name,
  • added them to syntax/help.vim,
  • changed green to limegreen for diffAdded in GUI,
  • changed blue to dodgerblue for diffChanged in GUI.

Light background:

Capture.d.ecran.2024-01-07.a.10.00.29.png (view on web)

Dark background:

Capture.d.ecran.2024-01-07.a.10.01.14.png (view on web)

limegreen is darker and less saturated than green without being as dark as darkgreen, so it is more legible on white and less jarring on black.

dodgerblue is lighter than blue but not too bright so it works equally well on white and black.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1879999671@github.com>

Romain Lafourcade

unread,
Jan 7, 2024, 4:10:01 AM1/7/24
to vim/vim, Subscribed

limegreen is still too bright to my taste/eyes

image

Could you suggest a few alternatives from https://en.wikipedia.org/wiki/X11_color_names#Color_name_chart ?

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1880000147@github.com>

Maxim Kim

unread,
Jan 7, 2024, 4:20:56 AM1/7/24
to vim/vim, Subscribed

Interesting, in your screenshots limegreen is actually readable compared to mine. I guess font-size matters a lot here.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1880002528@github.com>

Maxim Kim

unread,
Jan 7, 2024, 4:35:02 AM1/7/24
to vim/vim, Subscribed

limegreen is still too bright to my taste/eyes
image

Could you suggest a few alternatives from https://en.wikipedia.org/wiki/X11_color_names#Color_name_chart ?

  • SeaGreen
  • ForestGreen
  • WebGreen

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1880005516@github.com>

Christian Brabandt

unread,
Jan 7, 2024, 8:41:29 AM1/7/24
to vim/vim, Subscribed

The header diffOldFile and diffNewFile link to DiffFile which resolves to Type which uses SeaGreen, so let's use this one for DiffAdded as well.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1880063239@github.com>

Christian Brabandt

unread,
Jan 7, 2024, 9:46:41 AM1/7/24
to vim/vim, Subscribed

I have made a few minor changes, which turned out to be even more involved:

  • Renamed those new groups to Added, Changed and Removed (I initially went with capitalizing the diffAdded to DiffAdded, but since hightlighting groups are not case sensitive, it causes problems for linking the groups in the color schemes, so I had to make the name different and removed the diff prefix)
  • Changed the links in the default colorschemes
  • linked those new groups back in the diff syntax file.
  • capitalize the color names consistently

That is what I have temporarily queued, not sending out yet, would like to get an okay for that.:

chrisbra@0fea0ab

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1880079459@github.com>

Gary Johnson

unread,
Jan 7, 2024, 4:07:02 PM1/7/24
to reply+ACY5DGH5F224BM3O4Q...@reply.github.com, vim...@googlegroups.com
On 2024-01-07, Christian Brabandt wrote:
> I have made a few minor changes, which turned out to be even more involved:
>
> • Renamed those new groups to Added, Changed and Removed (I initially went
> with capitalizing the diffAdded to DiffAdded, but since hightlighting
> groups are not case sensitive, it causes problems for linking the groups in
> the color schemes, so I had to make the name different and removed the diff
> prefix)
> • Changed the links in the default colorschemes
> • linked those new groups back in the diff syntax file.
> • capitalize the color names consistently
>
> That is what I have temporarily queued, not sending out yet, would like to get
> an okay for that.:
>
> https://github.com/chrisbra/vim/commit/0fea0abf5a2e4a9b91df9b1c0084d42e9a44f82d

I tried cloning your repo without pulling down everything,

$ clone --depth 3 --no-single-branch https://github.com/chrisbra/vim.git

but it didn't include that commit. The latest relevant commit
appeared to be cdd869fd9, so I checked that out and built it. The
result looks pretty good on an xterm, tested with the following.

$ git diff cdd869fd9^ cdd869fd9 | VIMRUNTIME=runtime src/vim -N -u NONE -i NONE --cmd 'set rtp=runtime' -c 'syntax on' -

Regards,
Gary

vim-dev ML

unread,
Jan 7, 2024, 4:07:20 PM1/7/24
to vim/vim, vim-dev ML, Your activity

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/13825/c1880172003@github.com>

Christian Brabandt

unread,
Jan 7, 2024, 5:02:13 PM1/7/24
to vim/vim, vim-dev ML, Comment

but it didn't include that commit.

You would have to checkout branch gh-13825, e.g.: https://github.com/chrisbra/vim/tree/gh-13825

Let me include it now, we can always further re-adjust

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.Message ID: <vim/vim/pull/13825/c1880189231@github.com>

Christian Brabandt

unread,
Jan 7, 2024, 5:33:53 PM1/7/24
to vim/vim, vim-dev ML, Comment

Closed #13825 via 124371c.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.Message ID: <vim/vim/pull/13825/issue_event/11412118776@github.com>

Maxim Kim

unread,
Jan 7, 2024, 5:58:10 PM1/7/24
to vim/vim, vim-dev ML, Comment

Default colors for Added/Changed/Removed are good enough for most of the bundled colorschemes.

The only outlier was murphy where green is the normal fg color and it gets in the way with Added, so I changed it to white:

image.png (view on web)

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.Message ID: <vim/vim/pull/13825/c1880205934@github.com>

Maxence Weynans

unread,
Jan 7, 2024, 6:43:05 PM1/7/24
to vim/vim, vim-dev ML, Comment

Do we have a reliable, unequivocal way of pointing out to distro maintainers that some patches are fixes to existing features that don't work properly in the current major or minor version, so they don't rebase upon that without including those fixes ?

As we've seen with the colourscheme work that was pushed late into the vim 8.2 cycle, and all the fixes pushed early into the vim 9.0 cycle, or with this specific pull request, this is sorely needed.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.Message ID: <vim/vim/pull/13825/c1880220922@github.com>

Reply all
Reply to author
Forward
0 new messages