Re: [PATCH 7/7] diff-highlight: detect --graph by indent

From: Jeff King <peff@peff.net>
Date: Mar 22, 2018, 11:18 UTC
Message-ID: <20180322111826.GA22763@sigill.intra.peff.net>
In-Reply-To: <0e2317fc-cfec-ba7f-8ded-59cc6b7f9a63@talktalk.net>

On Thu, Mar 22, 2018 at 10:59:31AM +0000, Phillip Wood wrote:

Show 14 quoted lines

> > +sub handle_line {
> > +	my $orig = shift;
> > +	local $_ = $orig;
> > +
> > +	# match a graph line that begins a commit
> > +	if (/^(?:$COLOR?\|$COLOR?[ ])* # zero or more leading "|" with space
> > +	         $COLOR?\*$COLOR?[ ]   # a "*" with its trailing space
> > +	      (?:$COLOR?\|$COLOR?[ ])* # zero or more trailing "|"
> > +	                         [ ]*  # trailing whitespace for merges
> 
> Hi Peff, thanks for looking at this. I've only had a quick look through
> but I wonder if this will be confused by commit messages that contain
>   * bullet points
>   * like this

I think we should be OK because the commit messages are indented by 4 spaces, and we allow only single spaces amidst the graph-drawing bits (and we respect the "*" only in those graph-drawing bits).

So I think you could fool it with something like:

  git log --graph --format='* oops!'

or maybe even:

  git log --graph --format='%B'

but not with a regular git-log format. Those final corner cases I don't think we can overcome; it's just syntactically ambiguous. Unless perhaps we traced the graph lines from the start of the output and knew how many to expect, but that seems rather complicated.

Ultimately the best solution would be to avoid this parsing nonsense entirely by teaching Git's internal diff to produce the highlighting internally.

-Peff

PS I also eyeballed the results of "git log --graph -p --no-merges
  -1000" piped through the old and new versions. There are several
  changes, but they all look like improvements.