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

Configurable rendering of invisible characters like space, tab, and line feed #1068

Open
Omnikar opened this issue Nov 10, 2021 · 17 comments
Open
Labels
A-helix-term Area: Helix term improvements A-theme Area: Theme and appearence related C-enhancement Category: Improvements E-medium Call for participation: Experience needed to fix: Medium / intermediate

Comments

@Omnikar
Copy link
Contributor

Omnikar commented Nov 10, 2021

Some config option for whether to display invisible characters like rendering spaces as dots, tabs as arrows, etc.

The config option would have 3 possible values:

  • none: Don't render invisible characters.
  • selection: Render invisible characters that fall into a selection.
  • all: Render all invisible characters.

I presume this would also add a field to themes?

@Omnikar Omnikar added the C-enhancement Category: Improvements label Nov 10, 2021
@kirawi kirawi added A-helix-term Area: Helix term improvements A-theme Area: Theme and appearence related E-good-first-issue Call for participation: Issues suitable for new contributors labels Nov 11, 2021
@Rational-Curiosity
Copy link
Contributor

Rational-Curiosity commented Nov 15, 2021

Usually the most annoying invisible characters are those at the end of the line. In my opinion an option to highlight only these is very useful.

@Omnikar
Copy link
Contributor Author

Omnikar commented Nov 16, 2021

Perhaps the config option could allow things like this:

Render all whitespace

[editor]
render-whitespace = "all"

Render all newlines

[editor]
render-whitespace = { newline = "all" }
[editor.render-whitespace]
newline = "all"

Render all newlines, render other whitespace when selected

[editor]
render-whitespace = { default = "selection", newline = "all" }
[editor.render-whitespace]
default = "selection"
newline = "all"

@pickfire
Copy link
Contributor

pickfire commented Nov 19, 2021

We definitely could take some user experience from vscode, last time I used it setting this part they have some sort of presets, we could do that with what you showed.

[editor]
render-whitespace = "all"

And when customization is needed, we can can always fall back to

[editor.render-whitespace]
default = "selection"
newline = "all"

Thinking aloud, should we also let the users choose the characters of their choice like what you can do in vim?

Should we also allow theming this one? Since I sometimes see different foreground and background color for these texts, like in doom emacs?

@Omnikar
Copy link
Contributor Author

Omnikar commented Nov 19, 2021

Should we also allow theming this one? Since I sometimes see different foreground and background color for these texts, like in doom emacs?

Yeah, this definitely should be a theme thing.

@kirawi kirawi added E-medium Call for participation: Experience needed to fix: Medium / intermediate and removed E-good-first-issue Call for participation: Issues suitable for new contributors labels Nov 19, 2021
@Omnikar Omnikar mentioned this issue Dec 1, 2021
3 tasks
@the-mikedavis the-mikedavis mentioned this issue Mar 12, 2022
5 tasks
@mtoohey31
Copy link
Contributor

I would also like the option to only render trailing whitespace, i.e. spaces at the end of a line with no other characters following them.

@the-mikedavis the-mikedavis linked a pull request Apr 21, 2022 that will close this issue
@vwkd
Copy link
Contributor

vwkd commented Dec 9, 2022

In addition to trailing whitespace, VSCode has a useful option to show boundary whitespace, e.g. double space. Ideally, the option would also allow to show both trailing and boundary whitespace beside just one or the other.

@divarvel
Copy link
Contributor

divarvel commented Jan 8, 2023

Usually the most annoying invisible characters are those at the end of the line. In my opinion an option to highlight only these is very useful.

I have found "renderWhitespace": "boundary" from VSCode to be exactly what i want (and i patched kakoune to get it, unfortunately it never got merged).

@mtoohey31
Copy link
Contributor

mtoohey31 commented Jan 11, 2023

@divarvel I gave implementing a boundary whitespace renderer a go; it's on this branch here: https://github.com/mtoohey31/helix/tree/feat/boundary-whitespace. If you happen to have time, would you mind trying it out and letting me know if it works the way you'd expect? To enable it, running :set whitespace.render boundary should be all that's necessary.

@divarvel
Copy link
Contributor

divarvel commented Jan 11, 2023

Wow, thanks! I gave it a go and it works as documented indeed. I wonder if your work could be adapted to provide support for trailing as well, as it might be the most useful for people (reading the VSCode docs, i realized that trailing made more sense to me than boundary).

I can have a go at it and see how it goes.

@divarvel
Copy link
Contributor

divarvel commented Jan 11, 2023

The main issue with trailing is that it requires a double traversal of each line (one to know where the trailing whitespace starts, and one to actually draw graphemes), which might have a non-negligible perf impact.
One alternative solution would be to display the newline character only when the last char in an line is whitespace. This avoids requiring traversing each line twice as well as not interfering with how other whitespace is displayed.

Here's my attempt: divarvel@2ef08c1

2023-01-11-141109_580x154_scrot

pros:

  • it only affects newlines and doesn't interact with settings for other whitespace chars
  • it does not require traversing each line twice

cons:

  • it's a bit subtle and not always easy to spot trailing whitespace that is not otherwise displayed
  • it's different from what VScode does
  • it only affects newlines, even though it's possible to set trailing for spaces, tabs and nbsp as well, where it will be simply ignored

It depends on @mtoohey31's patch (which i believe only supports boundary for spaces) so i can't PR it as is.

@mtoohey31
Copy link
Contributor

Nice idea @divarvel, that's an interesting solution! The challenges you mentioned with trailing are actually why I decided to give boundary a shot first; I'd perfer trailing myself too. It might still be worth checking how much the double traversal solution would impact performance, because it might not be too bad. Or, we could give this kind of whitespace rendering a different name other than "trailing" and add it now, then if we come up with a performant implementation for what other editors refer to as "trailing", that could get added later under that name without changing the behaviour of an existing option.

I don't know if I'll get around to working on this again any time soon. Don't feel like you need to wait for me if you'd like to create a PR, if you want to create a PR based on my chanes that's fine with me. Not sure how the changes in #5420 will play with our changes though; I haven't checked but the mention of "rework rendering" in the title suggests that there might be significant conflicts.

@pascalkuthe
Copy link
Member

Nice idea @divarvel, that's an interesting solution! The challenges you mentioned with trailing are actually why I decided to give boundary a shot first; I'd perfer trailing myself too. It might still be worth checking how much the double traversal solution would impact performance, because it might not be too bad. Or, we could give this kind of whitespace rendering a different name other than "trailing" and add it now, then if we come up with a performant implementation for what other editors refer to as "trailing", that could get added later under that name without changing the behaviour of an existing option.

I don't know if I'll get around to working on this again any time soon. Don't feel like you need to wait for me if you'd like to create a PR, if you want to create a PR based on my chanes that's fine with me. Not sure how the changes in #5420 will play with our changes though; I haven't checked but the mention of "rework rendering" in the title suggests that there might be significant conflicts.

The text rendering code was completely replaced so your code would likely require significant changes.

@mtoohey31
Copy link
Contributor

Ok, that's alright. Thanks for your work on that PR by the way @pascalkuthe, I've been waiting for softwrap for a long time!

@wmstack
Copy link
Contributor

wmstack commented Mar 8, 2024

Is it possible to only show invisibles in normal mode? Or to have a mode-specific configuration for when invisible whitespace is shown? Plus, I want my cursor to change colour depending on whether the cursor is on a newline character.

I don't know about you, but because you can select both whitespace and newline, wanting to delete whitespace, you can accidentally delete a newline, and I find that I want the cursor to just change the colour if I am on a newline character so I know whether or not I want to delete it. To me, this strikes a balance between not being able to select a newline character (vim) and being able to know you're on a newline.

Finally, this is a bit tangential, but I find the "soft-wrapped newline" symbol kind of unnecessary in soft-wrap. This is for a different issue, but I don't really want to know that a line is soft-wrapped because line numbers already show that.

@Zoybean
Copy link
Contributor

Zoybean commented Mar 9, 2024

Regarding the "soft-wrapped newline" symbol, you can configure that to any string you like using editor.soft-wrap.wrap-indicator in your config.toml. I personally like seeing it, but if you want to turn it off, change it to the empty string.

@jean-dao
Copy link

I too would like an option to show trailing whitespaces only, because showing all whitespaces adds a lot of clutter. I quite like the solution proposed by @divarvel, also I reimplemented it on master if someone else is interested: jean-dao@3d0e232 (there's also an additional commit to have a separate style for newlines jean-dao@b026823)

@Zoybean
Copy link
Contributor

Zoybean commented Jun 23, 2024

@jean-dao I like this! would you make a PR for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements A-theme Area: Theme and appearence related C-enhancement Category: Improvements E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet