Persist "Hide whitespace" preference across PRs

[madisonscott]

Select Topic Area

Product Feedback

Body

Hey team, thanks for all your hard work!

What are your thoughts on persisting the "Hide whitespace" preference in diff view settings across PRs, as the Unified/Split preference is persisted? I pretty much turn "Hide whitespace" on for every diff I look at, so it'd be nice to only ever do that once :)

Upon searching for similar discussions, I found an older thread that mentioned that this setting is intended to store your preference per-diff. To maintain that functionality, "Hide whitespace" could be added as a user-level setting, defaulting to that value when looking at a PR for the fist time, then still allowing it to be set a t a per-diff level on the diff itself.

Thanks for considering!

[alexander-dermicus]

Note that this proposal already has a lot of support in the older thread: https://github.com/orgs/community/discussions/5486

[EskeRahn]

Personally I believe it would be even better if it was possible to set per filetype, as not ALL filetypes has whitespaces as pure cosmetics.

But ignoring whitespaces should also be coupled with not seeing solo { and } with a particular indentation as a good match for synchronisation. that causes bad syncs for e.g. C# and similar.

[ardicoetzee]

Yes, please add this, at least on a per-user level.

[mlafleur]

At a minimum, the "Show Whitespace" toolbar button should always be visible. It shouldn't be markedly easier to show whitespace than to hide it.

[pcnova]

THIS HERE! 🙏🏼

[cupcakearmy]

Reviewing code is a BIG part of the time i spend on github. Hiding whitespace is such a vital feature, i don't unterstand how this cannot be a priority. You already have the functionality, just persist it 🙏

[NicholasMTElliott]

My workflow literally involves manually turning on this flag on every single PR I do, because the vast majority of files I review the white-space does not matter.

[patrickholley-asurion]

It's amazing that a website literally for computer programmers can cause so much disruption in a programmer's flow. Please add this as a persistent option, it is annoying and slows me down having to check it every time.

[Rylab]

One more vote to please oh please make this a persistent user-level (and/or repo-level) setting! It's quite rare that whitespace changes matter in our workflows; swapping the default to ignore it, would save so much time.

[ardicoetzee]

A friend directed me to this as an interim solution: https://chromewebstore.google.com/detail/git-well-soon/ehpeaofieafibmhiagianfjjblpnmbdo

I.e. A chrome plugin that does exactly what we want!

[alexander-dermicus]

Great workaround! Thanks for sharing!

[atkeranov]

A dev could literally implement this in a day at max. Now imagine how many human work hours are wasted by developers coming here to read through this thread, vote, and comment.

[EskeRahn]

My guess would be that it is not the work load of the hack that is the issue....

IMHO implementing it as suggested surely would be better than what we have currently for many (including me), but really would be a inadequate hack/workaround for a deeper problem in
a) distinguishing logic and cosmetic changes, and
b) finding good ways to display these differences.

For the code part of say a .cs file spaces/indentions would be purely cosmetic, but inside a string value in the same file they might be important, with undesired nasty side effects, and thus not ignorable. and really ought to be visible to a reviewer, to catch a crucial system breaking typo!

So to do a logical diff without risk of false positives or false negatives some 'compiling' would be needed to decode each file into its logical components, including first detecting its programming language correctly (relying on the file extension might not be enough), before a compare can be safely done.
Maybe by the help af a compiler / de-compiler pair??

...And next comes thinking up a clever way of actually displaying the found logical differences between two files that char by char seems very different, but are close to logically identical (as we currently have it with spaces, but could be different line splittings as well)

So certainly not something pulled off in a single day....

[drwharris]

My guess would be that it is not the work load of the hack that is the issue....

IMHO implementing it as suggested surely would be better than what we have currently for many (including me), but really would be a inadequate hack/workaround for a deeper problem in a) distinguishing logic and cosmetic changes, and b) finding good ways to display these differences.

For the code part of say a .cs file spaces/indentions would be purely cosmetic, but inside a string value in the same file they might be important, with undesired nasty side effects, and thus not ignorable. and really ought to be visible to a reviewer, to catch a crucial system breaking typo!

So to do a logical diff without risk of false positives or false negatives some 'compiling' would be needed to decode each file into its logical components, including first detecting its programming language correctly (relying on the file extension might not be enough), before a compare can be safely done. Maybe by the help af a compiler / de-compiler pair??

...And next comes thinking up a clever way of actually displaying the found logical differences between two files that char by char seems very different, but are close to logically identical (as we currently have it with spaces, but could be different line splittings as well)

So certainly not something pulled off in a single day....

The functionality exists already. The only thing being asked is for a users to be able to have their preference to have it ON by default be remembered.

[EskeRahn]

@drwharris, Did you even read what I wrote, and what it was a comment to?

[drwharris]

I did and you have a valid point, however I use this existing functionality a lot and I use it knowing these limitations. And while a small inconvenience, I would like the option to default it to on.

[EskeRahn]

So you might have noticed that I started by saying that it for me too would be better than what we got now....

[johngallagher]

Please implement this. The previous thread has many comments. I heard devs complaining about this 3 years ago. It's 2024 and you have the backing of Microsoft. Please implement this.

[nelsondude]

This would be amazing to have! How can we get this feature prioritized? I find myself reviewing large PRs way too often, only to realize that its 95% whitespace changes. My company has strict policies on chrome extensions so that is not a viable option unfortunately.

[rgallagherab]

+1 on request for this user-level setting to be added.

[erorcodes]

This is one of the most annoying daily things I do (and that says a LOT) so please please prioritize 🙏

[armadsen]

Yet another request for this. I've written my own AppleScript to add ?w=1 to the URL with a keyboard shortcut in Safari, but it sure would be nice if persisting this were a GitHub feature.

(AppleScript below if anyone wants it. I use the excellent FastScripts to give it a keyboard shortcut.)

tell application "Safari"
	set currentDoc to front document
	set theWindow to window 1 where its document is currentDoc
	tell (current tab of theWindow)
		set URL to URL & "?w=1"
	end tell
end tell

[ybentz]

Another workaround to make it a one-click action. I wrote a tiny bookmarklet that simply adds the w=1 query param to the existing url and reloads the page. Much faster than using the Github UI although still requires doing it for every PR so obviously not ideal.

Save this as the url of a normal bookmark and name it whatever you like. When you're on a GH PR simply click the bookmark to apply:

javascript:(function()%7Bconst url %3D new URL(window.location.href)%3Burl.searchParams.set("w"%2C "1")%3Bwindow.location.href %3D url.toString()%7D)()

[madisonscott]

!!!! Thank you for sharing! I love how simple this hack is and how obviously trustworthy it is 👏

[fmichaut-diff]

Having to turn this on every single PR is ridiculous.
We have appearance settings for the tab-space, default emoji skin color, but not for an actually useful setting like "Hide PR Whitespace by default" ?

Or put it in accessibility idk, my point is we already have user settings that auto-set default values for when we are writing comments on PRs, so why can't we have this one which already has a button for it. Its not like we are asking for a new feature, just for a setting to enable it by default !

[pixelscript]

I personally think it should be a project level setting, but persisting across PRs or making it a single click would be a good alternative.

[EskeRahn]

Though I use this hiding whitespace-differences all the time, I DO understand why they hesitate with making it too easy to enable...

We all want this to be a reliable tool, but we must remember that not all whitespaces are ignorable! So it could give a nasty case where a major system have some breaking changes that are not seen due to this, reviewing a PR, and later for trying to debug the changes. And they risk getting a lot of bad publicity from such a case.

They want us to do it as an informed choice, knowing the risks, so we not just blindly use it!

A much better (and a LOT more complex solution) would be a logical compare, that could even catch simple renames of identifiers. But that would require the file to be interpreted/compiled in a suitable matter.

Currently we especially struggle in languages with block encapsulation chars like { and } that a single brace on a line with the right indentation is seen as a good match for synchronisation, where it seldom really is, and this is typically the real culprit in a compare looking garbled!

So perhaps a safer - yet text-only - option would be if it detected (and displayed) a block (including subblocks) with changed indentation as one difference only.

Or (as suggested elsewhere) when comparing texts, any line only containing one or more of
( ) [ ] { } ;
as the only thing on a line, should have this logically appended to the previous line for doing the sync/compare. This would get rid of most of the false-sync hits between two version of some code.

[luchillo17]

Hmm, I thought spaces within strings were a given to be untouched, we're talking about whitespace changes within code, afaik I have never seen a ignore whitespace changes that would also ignore them within strings or multiline strings (backtick strings ` in JS), neither in GH or in GitKraken where I can set the diff tool to always ignore whitespaces.

[EskeRahn]

...To do that it has to do some rough compilation handling inline strings, here an arbitrary example, where some spaces matters and others do not. I do not know if they handle this correctly already.

   string xxx = $"somthing { foo } something { bar } something { ( useFirst ? "the first" : "a following" ) } something ";

[NicholasMTElliott]

Sure; but we already have the feature to enable ignoring whitespace. Whatever that behavior is, that's sufficient for the vast majority of us. I've run into maybe one situation in the last 20 years where there was a bug because of whitespace being hidden in a code review. However, I have to manually turn on this feature to ignore whitespace changes on several PRs every day. If I'm in a situation where the whitespace finally matters... I will turn it off, do that review, and call it a day.

[luchillo17]

Indeed, the issue is not if it works properly, the issue is that it exists and we want to enable it either globally or by project.

[EskeRahn]

Do not get me wrong, it would certainly be better with even a global switch than what we got now. I'm just saying that I would hesitate too if I had to decide, as this is just a workaround (even with caveats), and not a solution to the issue. And I would suggest that the workaround was bound to filetypes, as this would rule out the worst caveats.

[mark-langer]

@willsmythe since you were involved in the original thread: Is there anything you guys could do to raise the priority on this?

[logic01]

Wow, 7 years later and we all still dream.

[cduflo]

Copying over from the original thread asking for this feature, I wrote a small chrome extension that handles this as it doesn't seem like GH is going to provide this feature - https://chrome.google.com/webstore/detail/git-well-soon/ehpeaofieafibmhiagianfjjblpnmbdo

https://github.com/orgs/community/discussions/5486#discussioncomment-4656459

[cduflo]

Thanks for taking the time to send some feedback, @fmichaut-diff ! Unfortunately what you are proposing - while a great idea - requires manifest host_permissions - which gives users a warning like: “Read and change all your data on the websites you visit”. I just recently refactored to remove that permission to put users more at ease, but it takes away the extension's ability to implement your proposal. I'll add it as an issue on the project and maybe there's a loophole I'm overlooking https://github.com/cduflo/gitwellsoon/issues

[acidoxee]

I'd also like to have reduced flicker if possible. With Refined GitHub also installed, PR pages sometimes reload two or three times when navigated to, with quite a lot of layout shift. And since GitHub isn't really loading fast these days, it's really not a good experience 😕

[cduflo]

Fair enough - seems like there's more uptake for this mode than I thought. I've had some more ideas overnight, I'll keep this issue updated as I have time in the coming week to address this. Thanks for the input 😃
cduflo/gitwellsoon#3

[cduflo]

v1.1.0.0 of the extension is out now, hopefully you notice the improvement. It was achievable without enhancing the permissions requirements, which was nice! Please let me know if you encounter any adverse behavior post-update: https://github.com/cduflo/gitwellsoon/issues

Please give the extension some stars if you think it deserves them https://chromewebstore.google.com/detail/git-well-soon/ehpeaofieafibmhiagianfjjblpnmbdo 🌟

[acidoxee]

I can confirm the double reload is gone, there's just the one that Refined GitHub introduces now (which I might opt out of despite the benefits since it's really bothering me). No issues with it until now. Thanks a bunch!

[ken-ailevate]

Another vote to persist 'Hide whitespace' the same way as 'Diff view'.

[taylorham]

Is anyone from Github out there? Anybody listening to this at all? I've been assuming this was on the radar for implementation for more than half a decade given how crazy easy it would be to persist just like the "Split vs. Unified" view option.

How are we still begging for this? Why is no one providing any kind of response?

[ozturkkl]

+1 here, it's incredibly frustrating for something like this to go back to the default setting and having to change it everytime there's a new PR to reveiw.

Why is this still not fixed after YEARS of ask...

[roryap]

I just spent a while looking at a changed C# file which was showing hundreds upon hundreds of changes--it was chaos. Then I hid whitespace and realized the only change was to convert from block-scoped namespace to file-scoped, which can be done with a quick action to any C# file without even knowing that it could cause reviewers potentially hours of befuddlement.

Please fix this.

[pmarshalla]

+1 here , this simple feature still hasnt been added ? its very frustrating having to do this on EVERY PR!!!! Please change.

[luchillo17]

I'm using the Refined Github extension and it helps that it puts the Hide whitespaces button directly on top of the diff, saves me at least 1 click and the popover animation time...

[ardicoetzee]

There is a slight improvement. But I don't know why they didn't just fix it properly when they made this change.

Now, if you check "Hide whitespace", then at least it remains that way for that project.

[ggalmazor]

Since August 2021 (when the original discussion for this was created), I've been involved in more than 3.3k PRs on GitHub, and I've always required enabling Hide whitespace either manually or through a browser extension. Being able to set this as a default for my projects/organizations/user would save me a lot of time in the aggregate.

The available browser extensions I've tried for this purpose don't eliminate the pain of this interaction, often requiring page reloads.

This is a feature in many GUIs:

Visual Studio Code

JetBrains IDEs

Please provide an option to set a default for the Hide whitespace diff option for a scope wider than a single PR, per project, organization, or user. An option to set the default per user would make the most sense to me personally, but any additional scopes for this default would be a considerable improvement at this point.