On Tue, Mar 20, 2018 at 11:58:14AM -0400, Jeff King wrote:

> The issue bisects to 7e4ffb4c17 (diff-highlight: add support for --graph
> output, 2016-08-29). I think the problem is the "\s+" at the end of the
> $GRAPH regex, which soaks up the space for the context, and accidentally
> treats the "-" line as a preimage removal.
> 
> But just switching that to "\s" doesn't quite work. We may have an
> arbitrary number of spaces between the graph ascii-art and the diff.
> E.g., if you have a commit at the base of a branch (the test in
> contrib/diff-highlight shows this case).
> 
> So I think you'd have to record the indent of the previous hunk header,
> and then make sure that the indent matched that. But even there, I think
> we're subject to false positives if a commit message contains a hunk
> header (it's indented an extra 4 characters, but we'd accidentally soak
> that up thinking it was graph indentation).
> 
> To make it bullet-proof, I think we'd have to actually parse the graph
> structure, finding a "*" line and then accepting only an indent that
> matched it.

Wow. Nerd snipe successful. This turned out to be quite tricky, but also
kind of interesting.

Here's a series which fixes it. The meaty bits are in the final commit;
the rest is just preparatory cleanup, and adding some tests (all are
cases which I managed to break while fixing this).

  [1/7]: diff-highlight: correct test graph diagram
  [2/7]: diff-highlight: use test_tick in graph test
  [3/7]: diff-highlight: prefer "echo" to "cat" in tests
  [4/7]: diff-highlight: test interleaved parallel lines of history
  [5/7]: diff-highlight: test graphs with --color
  [6/7]: diff-highlight: factor out flush_hunk() helper
  [7/7]: diff-highlight: detect --graph by indent

 contrib/diff-highlight/DiffHighlight.pm       | 89 +++++++++++++++----
 .../diff-highlight/t/t9400-diff-highlight.sh  | 81 +++++++++++++----
 2 files changed, 133 insertions(+), 37 deletions(-)

-Peff

Reply via email to