Re: [PATCH 1/1] Add feature to show log as a tree
- Date: Mon, 4 Mar 2019 10:13:23 +0100
- From: Micha Nelissen <nelissen.micha@xxxxxxxxx>
- Subject: Re: [PATCH 1/1] Add feature to show log as a tree
On 03-03-2019 14:16, Junio C Hamano wrote:
I am not sure what you mean by a "tree". It definitely is not a
tree perceived by Git users (which is what represents a directory
structure), and abusing the established term is confusing.
I called it tree because the output looks like (right half) of a tree.
I'm open to alternatives though.
There is no inherent ordering among children of a given commit, so
you'd invent some way to define what "the first child" is. That
much can be read from the above, but what is missing is why do you
even need to bother. What benefit do users get by identifying "the
first" child and cutting all others off---that is the necessary
justification for this change, which is missing from the above
Right, in my mind the problem statement is so clear and I didn't write
it down. There should have been an introduction paragraph.
Problem I want to solve. The --graph output is useful but often
unreadable, due to the many parallel lines. On the other hand, with the
default straight line log output all structure is lost. I am trying some
middle ground to see merge structure, but keep history readable. I
noticed in particular the branch point causes a line to go around or go
in between many commits leading to clutter, so I want to remove these.
By itself the exact point branched from is rarely important (at least to
me). And if it is, then there still is --graph :-).
"A (depth) level" is poorly explained---in fact, it is not explained
what it means at all. "We designate the first child of each commit
by some magic" is all we can read from it.
When processing the commit nodes. Start with depth level 0. Given some
current depth level D, assign depth level D, D+1, ... D+N-1 to the N
parents of a (merge) commit. Recurse.
In the output, the depth level is more or less the column number the
star '*' is in. (Although after merges of merges, there will be gaps in
the level numbering.)
After explaining "some magic" in a more meaningful way, perhaps
you'd realize that you'd want to stop saying "first child". It
OK well, the term "first parent" is already in the man page, right?
--first-parent. So the first child of a commit is the commit C among all
children H, such that when all of H are traced back to a common point S,
the trace line from C is the one that ends being the first parent of S.
ignore all side branches and concentrate only on the trunk of a
tree). And I'd probably call each child that is on the "trunk" line
of the most important lineage "primary child" or something like
Sort of, but remember this works recursive, there can be merges of
merges. Therefore the output is not a single trunk line. In other words,
the above commit S doesn't need to be on the trunk (or, '*' completely
left in the graph output).
In any case, if this is to sit next to and be friends with
"--topo-order", the option should be named as "--some-order".
The option name "--tree" (or "--blob", "--tag", or "--commit" for
that matter) is obviously unacceptable if the option is about
specifying how the commits in the "git log" output are ordered.
I see it as alternative to the --graph option. I also made it jump
there, but maybe gotos are rejected from style point of view. In this
case I thought it would be clear to see it as alternative to --graph.
It doesn't do justice in my opinion to call it --graph --micha-order (or
whatever ;-)), as it is not only about changing the order of commit
display. But again, I'm open to alternatives, I can make a personal
alias anyway :-).
Having said that, if you are omitting commits that are not on the
primary lineage of the history, then this is not "--anything-order"
option, in which case it may want to sit next to and be friends with
Ah no, it doesn't omit any commits. All commits are displayed.
Without clear explanation of what the change attempts to achieve in
docs and tests, there is no way to review to see if the new code is
How can I add tests for this feature? I'm also open to what kinds of
documentation to add, as I find it way easier to explain when you
actually run it to see the output it generates, than from abstract
paragraph of documentation text (like a manpage has). Some
mathematical-type of explanation like above is also not really
immediately enlightening, and also not necessary to use it I think.
Thanks for review, Micha