Add show-whitespaces feature
#2073
Conversation
|
One problem I see is that embedded code has empty spaces add to the start and end of each line.
A problem from GitHub's end, messes with copy and pasting code. |
|
Your fork has an obsolete tag, do you mind deleting it? git tag -d hotfix
git push --delete origin hotfix |
|
With the latest commit, I managed to enable showing new-line characters. I can't decide this yet, but is this a bit distracting?
|
This comment has been minimized.
This comment has been minimized.
This reverts commit 7cc8d9f.
|
Merging this because I want to see it in the wild and it's received enough reviews for now. These 2 suggestions would still be good though: |
|
Thank you for this feature ^_^ |
|
This is a great feature, but it's frustrating that it's enabled by default. I'm very used to reading code on GitHub, and right now don't have time to go hunting for what suddenly changed (but I did, and ended up finding it here after scrolling through the long list of GHR options one by one) to restore things to how they were. Perhaps this could be disabled by default, or provide a toggle at the top of code views to "show whitespace characters" (like the "show whitespace" option in PRs)? |
|
I wanted this feature too, but I expected to see an toggle button. 🤔 |
|
I'm personally fine with the whitespaces being made visible by default, but I'd rather retain the ability to select text including whitespace. With this feature, that's only possible if I start the selection in the non-whitespace part. Even if I'm not particularly interested in selecting the whitespace per se, the disabling of selections makes it hard to select text that is surrounded by whitespace, since I need to place the cursor exactly at the word boundary (e.g. the start of the non-whitespace part of a line) for the text selection to be enabled, which is definitely cumbersome. |
|
I don't necessarily mind this feature being enabled by default and yet more of a heads-up would have been nice. For instance—should #1137 have been bumped? |
|
I do mind this being enabled by default ... was very unexpected and imo it's just noise. |
|
@wearhere we don’t update that page that often, once a month usually |
|
I also came here because I found this distracting while reviewing PRs. I first disabled Refined Github to check whether it was Github or Refined that added it. Then I spent 10 minutes scrolling through the options to find out which one I should disable. Having a button and defaulting to off would be my choice #2240 |
|
I'm surprised to see internal whitespace made visible here. Usually I only see leading/trailing whitespace. Because of this, I had to disable the option. It made it too difficult to read. |
|
The author of this PR is surprised to see how many people find this feature distracting. The option to view whitespace per file was already mentioned in the original post of this PR, but was merged anyway because this PR had way too many reviews already and was already almost 2 months old, and assuming there would be future possible additions, and yes the author agrees of that. Meanwhile the author of this PR is
The author would like to mention the users that they are always welcome to disable this feature in the extension settings at any point, which is why this opiniated extension has added the ability to disable features in the first place because too many people had specific requirements. The author would like to get back to this issue as soon as they can and would also like to link to this xkcd comic hoping that the users would calm down. Feedback is always appreciated. Peace 🖖. |
10 participants
Epilogue
Before explaining how this feature works, I'd like to outline all the things I tried to implement this feature.
1. Custom Font
Just like @bfred-it in #588 (comment), I tried to create a custom font using FontForge adding glyps to the codepoints "Space" (
U+0020), "New Line" (U+000A), and "Horizontal Tabulation" (U+0009).As as expected it failed, the tab glyph was substitued by space glyph, and new line glyph was outright ignored. This looks like that the visual for control characters is handled by the application parsing the text and not by the font being used. That may be the reason no font in the world contains any glyph information placed in these codepoints, so the font-fallback ends up at the application handling that code point.
2. Font Ligatures
I have a feeling that this can be solved using font ligatures, providing glyphs information for how these whitespace characters interact with codepoints in the Basic Multilingual Plane (BMP).
I have no way to test this, I don't even know if this is possible, end of story.
3. Replacing Characters
The characters space and tab can be easily replaced with any of the codepoint in Unicode's Private Use Area plane. But as mentioned above, we loose semantics for control characters.
This PR
Closes #588.
With the above tests in consideration, I don't know if this feature can be implemented in any other way except for doing DOM manipulations. If there is I'm all ears.
This PR is a lot similar to #1989, doing a heck of lot DOM manipulations. At the start I had concerns about performance, but as it seems this feature does look a bit light that #1989.
Below are the list of characters and their substitution characters for reference.
White-space Characters
•U+2022•→U+2192→¬U+00AC¬Testing this PR
Also works on all lazy-loaded and demand-loaded diff and file views.
Possible Future Additions
Show white-space character in code blocks inside comments.
This is not a part of the current PR, as code blocks in comments are not separated into lines. Will see what I can do later.
Display every "invisible" (non-printable) character outside BMP using some method.
Generalizing this feature to
show-invisibles. Can be used especially to detect file corruptions. But this might be heavy on the browser (way too many DOM nodes).To show or hide white-space characters per file.
Screenshots