Ignore whitespace in diffs automatically

riophae commented 9 years ago

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 commented 9 years ago

Thanks for pointing out such an useful trick :clap: , 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:

if you got a getter idea, glad to change.

camsong commented 9 years ago

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

riophae commented 9 years ago

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

I just came up with an alternative approach worth considering.

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.
Turn on the function manually. (In my opinion,) it would be without any problems in most cases.
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?
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.
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 commented 7 years ago

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 commented 7 years ago

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

camsong commented 7 years ago

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

riophae commented 7 years ago

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

camsong commented 7 years ago

Released v0.16.3 to fix this

amit133 commented 7 years ago

I am not able to see this feature

Check https://github.com/amit133/KTAB/commit/0dbcf5a578b039af3fc8c1e918a4c6d58f522a99#diff-e4cd2a6be7b63a3b123aa8476c2456d1

ArtS commented 7 years ago

Is this ever going to be available in Pull Requests?

camsong commented 7 years ago

@ArtS @amit133 fixed in v0.16.5

roboweaver commented 7 years ago

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 commented 6 years ago

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 commented 6 years ago

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

camsong commented 6 years ago

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

phicharp commented 6 years ago

@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 commented 6 years ago

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

phicharp commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@ericarnold-granular Will consider about this