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. 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. 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. 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. Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.reviewboard.org Beanbag, Inc. - http://www.beanbaginc.com On Mar 25, 2013, at 1:53 PM, Stephen Gallagher <step...@gallagherhome.com> wrote: > The OpenLMI developers discovered a bug[1] today related to whitespace > display in the diff viewer. Specifically, whitespace changes are not > being shown, regardless of the setting of "Hide Extra Whitespace" or > "Hide Whitespace changes" toggles. > > This is a pretty serious regression, particularly when dealing with > reviews of Python code, which as I'm sure the Review Board developers > know relies heavily on accurate whitespace. > > Mostly I'm bringing this to the list to raise awareness of this bug in > the hopes that it will be fixed more quickly. > > > [1] http://code.google.com/p/reviewboard/issues/detail?id=2941 > > -- > 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.
