On 10/10/2019 12:13 PM, James Coglan via GitGitGadget wrote:
> This series of patches are designed to improve the output of the log --graph
> command; their effect can be summed up in the following diagram:
>
> Before After
> ------ -----
>
> *
> |\
> | * *
> | |\ |\
> | | * | *
> | | | | |\
> | | \ | | *
> | *-. \ | * |
> | |\ \ \ |/|\|
> |/ / / / | | *
> | | | / | * |
> | | |/ | |/
> | | * * /
> | * | |/
> | |/ *
> * |
> |/
> *
I took a brief look through your series, and I think this is a really
cool improvement. I use "git log --graph" all the time and those kinks
have bothered me, too.
I'd give you extra bonus points if your first patch added the case on
the left as an expected output and we watch it get simpler as you modify
the behavior in pieces.
I do really like how you isolated different transformations into
different commits. I would have given more specific feedback if I
was more familiar with graph.c. I'll need to play with it more to
give more substantive feedback.
The only thing I could complain about in the first glance is some
test file formatting stuff, like how you split a test case into
"setup" "create expected output" and "test the output". That would
be better as one test. Also you add a space between your redirect
character and your file ("> expect" instead of ">expect").
Those nits aside, I look forward to digging into the code more
soon.
Thanks,
-Stolee
