Show indentation changes in diffs.

Review Request #5351 — Created Jan. 29, 2014 and submitted

Information

Review Board
master

Reviewers

Show indentation changes in diffs.

We've always by default turned off showing inserts/deletes for lines
that only had indentation changes. This could make it hard to see that
those lines actually did change indentation, though.

We now show indentation marks to help indicate this. For indented lines,
a faint green series of > characters or ------>| strings are used on
the right-hand side of the diff to show indentation. If a line
unindents, then faint red < characters or |<------ strings are shown
on the left.

Sections of an equals block with indentation are treated like any other
modified section. They will always show up and won't be hidden under
collapsed sections.

In order to ensure indentation-only changes to files were shown, I had
to remove the logic in diff_file_fragment.html that determines if we
should show the fragment. We were only showing the table if we had
changed chunks or under other special conditions (such as binary files).
In practice, this logic wasn't great anyway, and ended up showing a
single line of border and shadow for such changes. We already had a
condition that showed "No changes were made to this file," which is a
better thing to show.

Created some test files that did indents, unindents, and had mixes of tabs
and spaces. Verified they looked correct and matched what my editor showed,
and that the tabs all stayed confined to their tab boundaries.

Added unit tests for all sorts of common and edge conditions. They passed.


Description From Last Updated

Any thoughts on using U+21E5 (rightwards arrow to bar) instead of >| ? You could then connect it to the …

daviddavid
david
  1. Does this address bug 1511?

  2. Show all issues

    Any thoughts on using U+21E5 (rightwards arrow to bar) instead of >| ?

    You could then connect it to the line using U+23AF instead of &mdash;

    1. That's actually what I used at first, and it was so tiny that I had to zoom in to tell what it was. So I replaced it with the ascii art. :/

      It also doesn't connect with U+23AF here at all, plus U+23AF ends up half-width and screws up all alignment. I can take a screenshot if you want to see it.

  3. 
      
chipx86
david
  1. One more question. Do the markers get included in selections?

    1. They do. I don't think there's any way to avoid that. Not that our selections are all that useful, since multi-line selections grab the other columns (grumble).

    2. Hmm. There are some non-standard CSS rules to make things unselectable, but I agree that it's probably not worth spending too much time on it until we get the column thing sorted.

  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Loading...