Make our diff parsing more flexible with regards to "Index:" lines.

Review Request #8851 — Created March 27, 2017 and submitted

Information

Review Board
release-2.5.x
1f2ed9f...

Reviewers

The "Index:" lines, often used in diffs to help give a stable filename
and a starting location to metadata for a diff, would historically come
right before a 67-character "====..." divider. However, this is not true
with all types of diffs. Some tools (such as WebStorm) like to inject
additional information in-between the "Index:" line and the divider, and
our parsing ends up ignoring the "Index:" line as anything special,
causing parsing issues for the remainder of that file's metadata, and
causing some of it to merge into the previous file or to be thrown away.

This change improves our parsing for this. Instead of assuming the
divider comes immediately after the index, we now look ahead for it. If
we find it before any unified diff data, then we accept the Index line,
and safely mark the location of the diff data. If we don't find the
divider, then we don't treat it as special.

This should help us to stay compatible with third-party diff tools that
inject custom metadata, without having to support each tool
individually.

All unit tests pass.

Description From Last Updated

Its probably worth replacing webstorm with something more generic. AFAIK all intellij ides are based off IDEA.

brenniebrennie
brennie
  1. 
      
  2. Show all issues

    Its probably worth replacing webstorm with something more generic. AFAIK all intellij ides are based off IDEA.

    1. Considered that, but I only have the diff output from this to sample with. I wouldn't want to assume and then find that there's some other difference in the diffs generated with another IDEA tool that I didn't account for.

  3. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (9187672)
Loading...