search

sureshvv / reviewboard

Automatically exported from code.google.com/p/reviewboard
0 stars 0 forks source link

Option to ignore trailing/inconsistent white space highlighting in diff #270

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
*NOTE: Do not post confidential information in this bug report.*

What's the URL of the page containing the problem?

What steps will reproduce the problem?
1. Make many white space changes to a file (like removing all trailing
white space)
2. Make only one substantive change in the file
3. Create a review request containing your changes.

What is the expected output? What do you see instead?
I'd like to be able to toggle white space differences, so you can see a
diff that doesn't show white space changes (diff -bBw).  Seeing them all
can clutter up the real change that was made.

What operating system are you using? What browser?
FF2/Linux

Please provide any additional information below.

Original issue reported on code.google.com by tvolkert on 23 Oct 2007 at 4:32

GoogleCodeExporter commented 9 years ago

Original comment by chip...@gmail.com on 23 Oct 2007 at 11:25

GoogleCodeExporter commented 9 years ago 
This sounds more like a problem with the diff upload tool, are you using 
post-review?

Original comment by segueton...@gmail.com on 3 Jun 2008 at 10:02

GoogleCodeExporter commented 9 years ago 
I'm using post-review, and it isn't a problem with the diff upload tool (though 
from
my wording I suppose it could be interpreted as such).

The display of whitespace differences is handled in
diffviewer/templatetags/difftags.py in the filter showextrawhitespace(). My 
request
is to make this highlighting of trailing whitespace optional. Right now I have a
local modification to my RB installation to turn this off. I know the philosophy
within VMWare is to go through and remove all of these trailing whitespaces from
their source, but this isn't realistic here.

Original comment by tremer...@gmail.com on 4 Jun 2008 at 2:00

GoogleCodeExporter commented 9 years ago 
I have done the same local changes to showextrawhitespace() + changed 
myersdiff.py from:
            if self.ignore_space:
                temp = line.lstrip()

                # We still want to show lines that contain only whitespace.
                if temp != "":
                    line = temp

to 
            if self.ignore_space:
                line = line.strip()

This removes most of the whitespace-only diffs. We do not really care about 
these
since removing them is not realistic in a big development group with multiple
operating systems and multiple editors.

Original comment by vesterb...@gmail.com on 3 Jul 2008 at 7:19

GoogleCodeExporter commented 9 years ago 
This option would be useful to us as well.

Thanks,
Nithin.

Original comment by nithinsu...@gmail.com on 18 Aug 2008 at 7:55

GoogleCodeExporter commented 9 years ago 
Pretty sure there's another bug with the same request somewhere.

We can probably put this in for 1.0.

Original comment by chip...@gmail.com on 18 Aug 2008 at 8:32

GoogleCodeExporter commented 9 years ago

Original comment by trowb...@gmail.com on 28 Sep 2008 at 9:03

GoogleCodeExporter commented 9 years ago 
I'm a newbie to using review board too. I'd like to 2nd having an option like 
this to
ignore the extra noise of white space.

Or perhaps, there is some ability to add additional DIFF options definitions in 
a
user's .reviewboardrc or a the system level. 

DIFFOPTIONS=xxx
DIFFCMD=yyyy/mydiff.exe

Again,

Original comment by btseb...@gmail.com on 16 Jan 2009 at 6:11

GoogleCodeExporter commented 9 years ago 
This might slip until after 1.0. I'd like to freeze the database schema soon 
and not
change it until after the release.

Original comment by chip...@gmail.com on 24 Jan 2009 at 12:18

GoogleCodeExporter commented 9 years ago 
I would like to second this proposal.  Trailing whitespace is pernicious, but 
in some
circumstances it's not feasible to remove it.

Having it come up in bright red is also visually distracting and as one poster 
says
it creates clutter and detracts from the actual changes being made.

Original comment by metatr...@gmail.com on 4 Feb 2009 at 12:01

GoogleCodeExporter commented 9 years ago 
"Having it come up in bright red is also visually distracting and as one poster 
says
it creates clutter and detracts from the actual changes being made." - 
Absolutely
agree. This is so annoying.

Original comment by mbasman...@gmail.com on 9 Feb 2009 at 4:15

GoogleCodeExporter commented 9 years ago 
There might be some work related to this post-1.0 for Summer of Code, and I
definitely want to introduce a new user prefs overhaul first. For the time 
being,
people can set a custom stylesheet to override in the browser or use one of the
GreaseMonkey or Stylish extensions floating around to disable the highlighting.

Going to push this out a bit.

Original comment by chip...@gmail.com on 6 Apr 2009 at 6:41

GoogleCodeExporter commented 9 years ago 
Is this really best as some kind of special user preference? While sure, there 
are
people who always want it on or always want it off, a lot of people I talk to 
want to
be able to see it but also want it to go away while they are looking at the 
actual
content. Perhaps a better way to handle this is something on the review page 
that
lets you toggle whitespace display on and off.

Original comment by psc...@vmware.com on 6 Jul 2009 at 11:36

GoogleCodeExporter commented 9 years ago 
Paul: We have a Summer of Code student actually working on providing whitespace
metadata that's available to the page such that individual users can toggle this
on/off when they want to. It'll give us a lot of flexibility on what we can 
allow
users to do here.

Original comment by chip...@gmail.com on 6 Jul 2009 at 11:45

GoogleCodeExporter commented 9 years ago 
I've just upgraded to 1.0 and have made use of the new ignore trailing 
whitespace
option for the diff viewer.  Very cool!

Original comment by metatr...@gmail.com on 27 Jul 2009 at 12:40

GoogleCodeExporter commented 9 years ago 
I guess that this functionality is pretty stable and works on all browsers. So 
shouldn't we set it to milestone 
release 1.1 instead of 1.5?

Original comment by eduardof...@gmail.com on 13 Sep 2009 at 9:30

GoogleCodeExporter commented 9 years ago 
Probably. I think this can actually be called fixed, too. Don't you think?

Original comment by trowb...@gmail.com on 13 Sep 2009 at 9:38

GoogleCodeExporter commented 9 years ago 
Well, I don't know the policy for marking something as fixed. Is it the code 
that fix the issue being on trunk or 
release? If it's on trunk then this can be marked as fixed.

Original comment by eduardof...@gmail.com on 14 Sep 2009 at 12:10

GoogleCodeExporter commented 9 years ago 
Trunk is fine for 1.1 milestoned things :)

Original comment by trowb...@gmail.com on 14 Sep 2009 at 12:12

GoogleCodeExporter commented 9 years ago 
In response to Christian's comment 14, I'm wondering if the individual diff 
option toggle is still under 
development?  The global options are a little too broad for me.  We're using 
review board version 1.0.5.1.

Original comment by eri...@gmail.com on 13 Mar 2010 at 8:53