Skip to content
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 additional ability to (optionally) decorate the Colorcolumn with a character and/or custom highlighting groups #11860

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jrahm
Copy link

@jrahm jrahm commented Jan 22, 2023

Hi all,

Several months ago I created this pull request for Neovim to add a new fillchar, colorcol. They recommended I create a pull request upstream to see if there was any interest. So after a few months, I finally got up the energy to give it a shot.

I use Vim's colorcolumn often, however, I prefer the look and feel of more recent editors where the "color column" is just a thin line. Highlighting a full block just feels ... inelegant. My solution to add this to vim, is to add a new fillchar, colorcol. This char is displayed in the colorcolumn using the colorcol highlighting. So ,for me, where I prefer the thin line, I set colorcol to , but others may decide to set it to , ~, or ., or , though I imagine most would probably set it to some flavor of the vertical box-drawing characters.

Here is an exmaple of how it works with basic configuration in what I imagine will be the typical usage:
colorcolchar_1

And if one is so possessed to use multiple colorcolumns with this option:
colorcolchar_2

Let me know what else might me needed. I tried to add a test and follow existing conventions, but may have overlooked something. Thanks!

@codecov
Copy link

codecov bot commented Jan 22, 2023

Codecov Report

Merging #11860 (6a5e226) into master (6ec6666) will decrease coverage by 0.02%.
The diff coverage is 70.00%.

❗ Current head 6a5e226 differs from pull request most recent head bf097bd. Consider uploading reports for the commit bf097bd to get more accurate results

@@            Coverage Diff             @@
##           master   #11860      +/-   ##
==========================================
- Coverage   81.89%   81.87%   -0.02%     
==========================================
  Files         164      164              
  Lines      193211   193169      -42     
  Branches    43781    43794      +13     
==========================================
- Hits       158231   158162      -69     
- Misses      22175    22182       +7     
- Partials    12805    12825      +20     
Flag Coverage Δ
huge-clang-none 82.60% <70.00%> (+<0.01%) ⬆️
huge-gcc-none 53.71% <70.00%> (-0.01%) ⬇️
huge-gcc-testgui 52.19% <70.00%> (-0.01%) ⬇️
huge-gcc-unittests 0.29% <0.00%> (+<0.01%) ⬆️
linux 82.31% <70.00%> (+<0.01%) ⬆️
mingw-x64-HUGE 76.46% <50.00%> (-0.04%) ⬇️
mingw-x86-HUGE 76.91% <50.00%> (-0.03%) ⬇️
windows 78.05% <50.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/drawline.c 84.00% <66.66%> (-0.09%) ⬇️
src/screen.c 80.38% <100.00%> (+0.01%) ⬆️
src/scriptfile.c 83.38% <0.00%> (-1.23%) ⬇️
src/cmdhist.c 89.81% <0.00%> (-0.81%) ⬇️
src/if_xcmdsrv.c 77.12% <0.00%> (-0.70%) ⬇️
src/os_win32.c 66.92% <0.00%> (-0.60%) ⬇️
src/cmdexpand.c 90.65% <0.00%> (-0.23%) ⬇️
src/highlight.c 80.93% <0.00%> (-0.12%) ⬇️
src/channel.c 83.05% <0.00%> (-0.09%) ⬇️
src/term.c 74.04% <0.00%> (-0.09%) ⬇️
... and 33 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@brammool
Copy link
Contributor

brammool commented Jan 22, 2023 via email

@jrahm
Copy link
Author

jrahm commented Jan 22, 2023

I like this idea. I thought it might be cool to have multiple colorcolumns with different highlights. One for maybe a "soft" textwidth recommendation and one for a "hard" textwidth (for example). I suspect it wont be too much trouble to implement. (Famous last words, amirite) So I'm willing to give it a shot.

I assume the syntax will be something like

colorcol     := <colorcolitem>,...
colorcolitem := <number>        // just use the number with ColorColumn formatting and a <space>' as the character
              | <number>/<character?>/<highlight?>  // Use number, character and highlight. If highlight is omitted, the ColorColumn is used, if character is omitted, then <space> is used.

ofc if someone is so possessed as to use a forward-slash for the character, it would need to be escaped.

I like this idea.

@habamax
Copy link
Contributor

habamax commented Jan 22, 2023

Imagine bigger text, or scroll 2 lines down and you've lost visual hint for the colorcolchar:

image

Other gui editors draw it in a different way, for example:
image

@jrahm
Copy link
Author

jrahm commented Jan 23, 2023

I understand the concern, and I appreciate you bringing it up, a couple of things:

  1. This PR doesn't change the existing semantics of the ColorColumn (at least it's not supposed to), just extends them to give the user more ability to decorate them how they may like. Especially after incorporating brammool's ideas (though this is still a WIP). Perhaps the name of my PR needs to be changed. I apologize if this was unclear.
  2. There would still be a visual cue. The letters in the colorcolumn are hl-combine'd with ColorColumn, so characters will inherit ColorColumn. But it would be an issue if all the characters were highlighted. But, sure, if the ColorColumn hlgroup were cleared, then there would be no visual cue, but that's already the case. (Though I personally, for my use case, wish the colorcolumn would just not display if a character was written over it, but that's my use case.)
  3. I think it should be on the user to use the proper ColorColumn configuration for their use-case, no? Afterall it's on the user to determine the proper ColorColumn column(s). Maybe I'm a bit myopic because I'm primarily a coder and so the colorcolumn indicating the rightmost margin is really my only real use-case for it, in which case, lines crossing the colorcolumn are sparse so the box drawing vertical line example is perfect. In the example above where the text crossing the colorcolumn is dense, then background highlighting is probably the way to go. I also never use GVim, only terminal vim, so maybe there's some sharp edges I'm not aware of.
  4. GUIs are always going to have an advantage when it comes to drawing arbitrary stuff on the window, but, at least in the terminal, I think this is a good compromise.

I'm happy to answer any more questions

@jrahm jrahm changed the title Colorcolchar Add additional ability to (optionally) decorate the Colorcolumn with a character and/or custom highlighting groups Jan 23, 2023
…lumn.

This commit add support for a new fillchar called 'colorcol'. When
colorcol is set, the colorcolumn will display that character in the
colorcolumn. The default value is ' ', which results in original
behavior.

Likely colorcol, if set to anything at all, will be set to something
like '│' or '┆' to provide a nice, thin line, but it is certainly not
limited to just those. Good ascii alternatives could be ':', '.' or '~'
(to match NonText).

In the case of multiple colorcolumn's the character is displayed in them
all.

If a typed character is in the colorcolumn, then the typed character
gets precedence over the colorcol char.
Color column arguments can now be configured as:

<number>[/<char>[/<hlgroup>[/<flags>]]]

where

 * <number> defines the color column as originally defined
 * <char> defines the character with which to fill the colorcolumn
   (defaults to ' ' to preserve semantics)
 * <hlgroup> defines the highlight group to use for that column
   (defaults to ColorColumn to preseve semantics)
 * <flags> defines additional flags for displaying the colorcolumn
   - has just 'b' defined, which displays the colorcolumn behind any
     text that goes over it.
   - no flags preserves semantics.
@jrahm
Copy link
Author

jrahm commented Jan 24, 2023

PTAL

Here's a couple of example of how I might use this new feature:

colorcol_ex1

here this is set with set colorcolumn=80/┆/ColorCol2/b,100/│/ColorCol3,60/\ /ColorColumn,4

colorcol_ex2

This is set with a script that add a colorcolumn at column 4, one at 80, and then ones at every 4 columns from 4 to 80

@brammool
Copy link
Contributor

I have my doubts about trying to put all of this into one option value. It can work for maybe up to three items, but above that it quickly gets messy. And if we get more requests later it's going to be hard to extend. We could keep 'colorcolumn' simple and add a function to set more complicated configurations. Then a dictionary can be used that's easy to extend with more entries later. When using a script to set this a function will be easier to use as well.
Considering that not much comments are given here it doesn't sound like an important feature to add (compared to all the other features that have been requested).

@yegappan yegappan added this to the backlog milestone Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants