Page MenuHomePhabricator

Diffs that have had trailing whitespace trimmed or were generated with the Git "diff.suppressBlankEmpty" config option render oddly
Open, WishlistPublic

Description

Normally, lines of a diff begin with (space; line unchanged), + (line added), or - (line removed).

If an empty line is present in a diff and was not modified, it will normally appear as a line containing a single space in the diff.

Git has a diff.suppressBlankEmpty config option which suppresses this space and makes unmodified, empty lines render as a single newline in the diff output instead.

These diffs are parsed and displayed oddly, in a misleading/confusing way. Ideally, desired behavior would be to parse these diffs as though they still has the spaces. If this turns out to be tricky or create ambiguities for whatever reason, an explicit parsing error would be desirable.

Slightly-approxiamte reproduction case:

  • Run git -c diff.suppressBlankEmpty=true diff -U99999 HEAD on pretty much any modified file with unchanged, empty lines.
  • Copy/paste the resulting diff into the web UI.
  • Get a mess in Differential with muddled rendering like this (note that line "59" on the left is three lines tall).

Screen Shot 2019-10-23 at 7.42.44 PM.png (90×2 px, 29 KB)

Event Timeline

epriestley triaged this task as Wishlist priority.Oct 24 2019, 2:43 AM
epriestley created this task.
epriestley renamed this task from Diffs that have had tailing whitespace trimmed or were generated with the Git "diff.suppressBlankEmpty" config option render oddly to Diffs that have had trailing whitespace trimmed or were generated with the Git "diff.suppressBlankEmpty" config option render oddly.Oct 24 2019, 2:43 AM

One change we can make here is to explicitly specify -c diff.suppressBlankEmpty false in the arc diff command, as we currently use --src-prefix and --dst-prefix to override diff.mnemonicprefix. This is likely desirable, but it would be good to also parse these diffs properly if they arrive through --raw or copy/pasting, etc.

After D20877, we'll forcefully override this option when generating diffs internally.

If you manually git diff with this option, then copy/paste the diff into the web UI, we'll still get things wrong. Hopefully, this is rare. Some future parser change will attempt to improve this.