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

Ignore whitespace in diffs automatically #5

Closed
riophae opened this issue Apr 13, 2015 · 33 comments
Closed

Ignore whitespace in diffs automatically #5

riophae opened this issue Apr 13, 2015 · 33 comments
Assignees
Labels

Comments

@riophae
Copy link

riophae commented Apr 13, 2015

How about adding ?w=1 to any diff URLs automatically as optional? Personally speaking it's pretty useful, as I hardly ever care about any changes only in whitespace.

@camsong
Copy link
Owner

camsong commented Apr 13, 2015

Thanks for pointing out such an useful trick 👏 , I'm sure it will help a lot of people.
In order not to confuse people who has got used to the current diff content, maybe we can add a button to enable this magic like this:

image

if you got a getter idea, glad to change.

@camsong camsong self-assigned this Apr 13, 2015
@camsong
Copy link
Owner

camsong commented Apr 13, 2015

A diff which may confuse people with this trick without a hint
https://github.com/angular/angular.js/pull/10539/files?w=1

@riophae
Copy link
Author

riophae commented Apr 14, 2015

Well done! 👍 Blazing fast!
Yes, indeed. In specific cases it would be very confusing.

I just came up with an alternative approach worth considering.

  1. A checkbox in option page setting up whether ?w=1 will be added to any diff links automatically. The checkbox is unchecked as default, as users should be clear about the function before it being enabled.
  2. Turn on the function manually. (In my opinion,) it would be without any problems in most cases.
  3. With this trick, very rarely and unfortunately, the situation that no changes couldn't be shown is possible. And there is not a hint, which causes confusion. Okay. How about appending a hint into the page, leading user to the normal diff page?
  4. Even more advanced (and radically), user could be redirected to the page without ?w=1 automatically, then we can show a hint that why whitespace hiding is disabled.
  5. However, automatical redirecting is VERY RISKY. Please avoid of using it as far as possible.

May be a bit opinionated... or just all my fantasy :D
Thx for your patience!

@mattsfrey
Copy link

Is there any consideration for this still? Having an option to default PR's to not showing whitespace diffs would be extremely useful, especially for those of us writing clojure and aligning maps values, as an example. We get diffs that are often 50% or more just whitespace and have been having to manually append the ?w=1 to our PR URLS.

@camsong
Copy link
Owner

camsong commented Aug 11, 2016

Prepare to add this but don't get a time, PR is welcomed.

@camsong
Copy link
Owner

camsong commented Sep 15, 2016

Finally this feature comes out. Will toggle between Show Spaces and Ignore Spaces like
image

image

@riophae
Copy link
Author

riophae commented Dec 23, 2016

Seems it's not applied to commit details page? I didn't see a "Ignore Spaces" button there.

@camsong
Copy link
Owner

camsong commented Dec 24, 2016

Released v0.16.3 to fix this

@amit133
Copy link

amit133 commented Jan 6, 2017

I am not able to see this feature
image

Check amit133/KTAB@0dbcf5a#diff-e4cd2a6be7b63a3b123aa8476c2456d1

@ArtS
Copy link

ArtS commented Mar 26, 2017

Is this ever going to be available in Pull Requests?

@camsong
Copy link
Owner

camsong commented Mar 27, 2017

@ArtS @amit133 fixed in v0.16.5

@roboweaver
Copy link

The w=1 trick is nice for viewing diffs, unfortunately you can't add comments for the Pull Request which is where this would be most useful.

Really annoying to not be able to see your actual change in a file that accidentally (or intentionally) got formatted as part of a commit, since changing the whitespace doesn't matter in most languages.

@meisertedu
Copy link

Are team would get alot of use from the menu/button option for selecting "no spaces" in diff when doing PRs. One common use-case for us is Jenkins often changes spaces/tabs to files automatically... which then leads to lots of file diff segments that are not "real" diffs. This button suggestion above would be great in the PR Files Changed tab/section.

@phicharp
Copy link

phicharp commented Dec 5, 2017

Any time estimate for when this will finally make it?
Hopefully ignoring spaces will not ignore leading white spaces in python?

@camsong
Copy link
Owner

camsong commented Dec 6, 2017

Better to turn 'Hide Space' off in python code @phicharp

@phicharp
Copy link

phicharp commented Dec 6, 2017

@camsong : we are moving a rather old project to using autopep8 and many spaces are removed (old style was with spaces after and before parentheses, around "=" etc...) and it is quite painful to see "real changes" if any, so hiding spaces is nice, but of course leading spaces are a problem...

@camsong
Copy link
Owner

camsong commented Dec 6, 2017

@phicharp It's hard to support that. Right now, "hide space" is a feature supported by github, can not be modified.

@phicharp
Copy link

phicharp commented Dec 6, 2017

OK, I understand this is not easy to implement, but AFAIS it is not exposed in the web interface, is it? I manage by adding "by hand" ?w=1 to the URL but this is not really nice

@ericarnold-granular
Copy link

Just wanted to leave another vote to add a button to PR pages to do this. I learned of the trick using w=1, but wish there was a way to make that the default. I agree there are times when it should be flipped off, so having the button at the top of the page would be ideal.

Any chance of getting this promoted to a user pref setting that is applied automatically as the default state, then having the button to flip from there?

Thanks

@camsong
Copy link
Owner

camsong commented Dec 15, 2017

@ericarnold-granular Will consider about this

@glynhudson
Copy link

Agree, this should be default. Or at least a toggle option. Please make it happen 👍

@nadim
Copy link

nadim commented Aug 9, 2018

I love the checkbox that's been added for Hide Whitespace Changes. Can we add an option to make it the default?

@nicolas-albert
Copy link

@nadim I search and I didn't find the checkbox on a page with code changes, I have to add w=1 by hand ...
Where can I find it please ?

@nadim
Copy link

nadim commented Aug 22, 2018

@nicolas-albert
image

@nicolas-albert
Copy link

@nadim oops, I haven't seen that the report was for a Chrome extension :-Z
I have installed it now 👍
Thx !

@nadim
Copy link

nadim commented Aug 23, 2018

What chrome extension?

I'm pretty sure this is default github behaviour. No extension required.

@nicolas-albert
Copy link

On this kind of page, there is no button by default :
nicolas-albert/c8o_yaml_projects@bb851e5?w=1

I have installed Octo Mate to have the Ignore / Show Spaces button.

@lekova
Copy link

lekova commented May 24, 2019

@nadim do you know if that checkbox selection can be persisted? I find myself selecting it and then the page is reloaded, the selection is gone.

@mlandisbqs
Copy link

+1 for a persistent toggle - thank you!

@alamothe
Copy link

Is it supported to persist the setting across PRs?

@mustafaozhan
Copy link

mustafaozhan commented Jun 24, 2021

+1 for persistency, I want it to be saved as my user preference, so I will not have to enable it for each PR I am reviewing

@YannikBartel
Copy link

+1 for persistent

@dcumings
Copy link

dcumings commented Sep 6, 2022

+1 for persistency.
Found this in the short term:
https://chrome.google.com/webstore/detail/github-whitespace/fnpkdafamnbjoldglihkjjdicofghccm/related?hl=en-US

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests