That actually toggles the visibility of the big red trailing/mixed whitespace indicators (but not the lines). It's confusing and I'm really hoping to rewrite the UI for it for 1.8.
Christian On Mar 26, 2013, at 4:36, Stephen Gallagher <step...@gallagherhome.com> wrote: > On 03/25/2013 05:04 PM, Christian Hammond wrote: >> Hi Stephen, >> >> Whitespace for indentation has never been shown for most files, >> regardless of that setting. That's just due to how we've always >> generated diffs. >> >> During diff generation, indentation changes are ignored. The intent was >> to prevent a block of indentation changes from showing up as a bunch of >> modified lines or, worse, deleted/added lines, distracting from the >> content of those reviews. > > Sure, I understand that, but I would have figured that's what the "Hide > whitespace changes" button would have been for. As it stands right now, > those two buttons don't appear to do anything at all. > > >> >> All other whitespace changes are then let through (trailing whitespace, >> whitespace within the line), and lines containing just that are assigned >> a class. The "Show/Hide Extra Whitespace" toggle then just changes the >> visibility of any chunks containing these classes. It doesn't regenerate >> the diff or anything. The goal of this feature is to make code review >> easier when the file contains lots of whitespace cleanup, but not to >> show indentation. >> >> So, they're noticing this for the first time. It has never been any >> different, out of the box. > > Well, this instance has only been up for about a week, so "for the first > time" happened pretty quickly. > > >> This can be disabled in an installation, though. You can include *.py >> files in "Show all whitespace for:" list in the Diff Viewer Settings in >> the Admin UI, and we'll stop ignoring leading whitespace on lines. > > I couldn't get this to work for > https://reviewboard-openlmi.rhcloud.com/r/54/diff/ > > I added "*.py, openlmi-doc-class2*" to the ignore list in the Diff > Viewer settings (and restarted the server), but it still does not > display leading whitespace changes on line 124 of that diff. > > >> Can this all be better? Yes. What I'd love is to have the diff >> generation be much smarter and say "This line hasn't changed except for >> the indentation," and then mark that up differently. There's a lot of >> work that would be needed, though. Something to put on the increasingly >> large TODO list. > > The diff generation seems to be able to identify the specific characters > that have changed within a line other than whitespace, so I'm not sure > why we can't just use that same logic. But I'll admit to having no clear > view of the internal details here. > > -- > Want to help the Review Board project? Donate today at > http://www.reviewboard.org/donate/ > Happy user? Let us know at http://www.reviewboard.org/users/ > -~----------~----~----~----~------~----~------~--~--- > To unsubscribe from this group, send email to > reviewboard+unsubscr...@googlegroups.com > For more options, visit this group at > http://groups.google.com/group/reviewboard?hl=en > --- > You received this message because you are subscribed to the Google Groups > "reviewboard" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to reviewboard+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/groups/opt_out. > > -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~----------~----~----~----~------~----~------~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en --- You received this message because you are subscribed to the Google Groups "reviewboard" group. To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
