On Thu, Mar 22, 2018 at 10:59:31AM +0000, Phillip Wood wrote:
> > +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.
