Re: [PATCH 1/1] Add feature to show log as a tree

[Junio C Hamano <gitster@pobox.com>]

Micha Nelissen <nelissen.micha@gmail.com> writes:

> Modifies the topo-order code to keep track of the first child,
> using a heuristic. The heuristic works by assigning a (depth)
> level to all nodes. The first child is the node of which this
> node is a parent of and has the lowest level. Then it cuts all
> the links that are not the first child, resulting in a tree.

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.

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
explanation.

"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.

Please try again.

After explaining "some magic" in a more meaningful way, perhaps
you'd realize that you'd want to stop saying "first child".  It
feels to me that you want to use a heuristic to identify the most
important single line of history leading to the tip.  If that is
what you are doing, then the resulting shape of the history would
not be a tree (where branching is an important aspect, and both
sides of the branches are important) but a linear line (where you
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
that.

> It also uses the level to sort the nodes: trying to keep
> descendent line (of a merge) together as a group.
>
> Add commandline option "--tree" to use this new feature.

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.

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
"--first-parent", perhaps?  with the fuzzy description and
explanation above, I cannot quite guess the answer to this question
myself so I'll stop here.

> Signed-off-by: Micha Nelissen <nelissen.micha@gmail.com>
> ---
>  commit.c   | 136 +++++++++++++++++++++++++++++++++++++++++++++++------
>  commit.h   |   1 +
>  revision.c |   4 ++

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
correct.