On 1/19/21 5:10 PM, Junio C Hamano wrote:
Kyle Marek <kmarek@xxxxxxxx> writes:
So the condition we saw in your patches, !commit->parents, which
attempted to see if it was root, needs to be replaced with a helper
function that checks if there is any parent that is shown in the
output.
...
Hmm?
Okay, I see what you mean. Fixing --graph to avoid implying ancestry
sounds like a better approach to me.
Sorry, I do not know how you drew that conclusion from my
description.
All I meant to convey is "roots are not special at all, commits that
do not have parents in the parts of the history shown are, and care
must be taken to ensure that they do not appear to have parents".
Yeah, I guess I am confused. I thought "Fixing --graph to avoid implying
ancestry" was reaching the same point as "care must be taken to ensure
that [commits without parents shown] do not appear to have parents". (I
wasn't just talking about root commits at that point)
And the argument applies equally to either of two approaches.
Whether the solution chosen is
(1) to use special set of markers "{#}" for commits that do not
have parents in the displayed part of the history instead of
the usual "<*>", or
(2) to stick to the normal set of markers "<*>" but shift the graph
to avoid false ancestry.
we shouldn't be special casing "root commits" just because they are
roots. Exactly the same issue exists for non-root commits whose
parents are not shown in the output, if commits from unrelated
ancestry is drawn directly below them.
I understand. Coming back to the "root commit" situation below.
That being said, I spoke to Jason recently, and he expressed interest
in optionally marking root commits so they are easy to search for in a
graph with something like /# in `less`. I see value in this,
I do not mind to denote the "this commit may appear directly on top
of another commit, but there is no ancestry" situation with a
special set of markers that is different from the usual "<*>" (for
left, normal and right) set. I agree pagers are good ways to /search
things in the output.
So would you be open to my modifying of the patch in question (patch
1+2 squashed, I guess) to instead use "--mark-roots=<mark>" to
optionally mark root commits with a string <mark>, and pursue fixing
the --graph rendering issue in another series?
I do not mind if the graph rendering fix does not happen yet again;
IIRC the past contributors couldn't implement it, either.
I think this new feature should be made opt-in by introducing a new
option (without giving it a configuration variable), with explicit
"--no-<option>" supported to countermand a "--<option>=#" that may
appear earlier on the command line (or prepare your scripts for
later introduction of such a configuration variable).
Okay
I do find it troubling if the <option> has "root" in its name, and I
would find it even more troubling if the feature somehow treated
root commits specially but not other commits that do not have their
parents shown. It was the primary point I wanted to stress in the
previous two message [*1*].
I'll come back to this below.
I am hoping that a single option can give three-tuple that replaces
the usual "<*>", with perhaps the default of "{#}" or something.
I thought about that, but can we handle any of the three markers being
multi-byte characters?
I however offhand do not think of a way to make "left root" appear
in the output, but because we'd need "right root" that looks
different from ">" anyway, it may make sense to allow specifying
"left root" just for symmetry.
I'm thinking on that one. I need to learn more about --left-right. I
don't know how/when to use it yet.
[Footnote]
*1* But if we do not think of a good option name without the word
"root" in it, I might be talked into a name with "root", as long
as we clearly describe (1) that commits that has parents that
are not shown in the history are also shown with these letters,
and (2) that new contributors are welcome to try coming up with
a new name for the option to explain the behaviour better, but
are not welcome to argue that the option should special case
root commits only because the option is named with "root" in it.
So, on the root vs parents-not-shown commits issue:
You're right. Commits with their parents hidden by the range specifiers
have the same graphing issue as root commits.
While root commits are not a special case in the sense that --graph
makes ancestor implications for more than just root commits, root
commits are a special case when we think about interpreting the presence
of hidden lineage in --graph output.
Considering one of your examples:
C
/
O---A---B
\
X---Y---Z
When graphing C..Z, git produces output like:
* 0fbb0dc (HEAD -> z) Z
|\
| * 11be529 (master) B
| * 8dd1b85 A
* 851a915 Y
* 27d3ed0 (x) X
We cannot tell from the above graph alone that X is a root and A is not.
So I think it might be useful if I could do --mark-roots='#'
--mark-hidden-lineage=$'\u22ef' (Unicode Midline Horizontal Ellipsis) to
produce the following:
* 0fbb0dc (HEAD -> z) Z
|\
| * 11be529 (master) B
| ⋯ 8dd1b85 A
* 851a915 Y
# 27d3ed0 (x) X
Alternatively, it could be argued that --boundary can be used to
indicate a hidden lineage, since root commits do not have boundary
commits listed below them. But --boundary draws one more commit than
necessary, and there still isn't an easy way to search for roots in the
pager.
I understand that I am now leaving the original scope of the issue, but
I think it is worth considering.
Of course, I would also like to try fixing the original graphing issue
in general.
Thoughts?
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
- -
- Kyle Marek PD Inc. http://www.pdinc.us -
- Jr. Developer 10 West 24th Street #100 -
- +1 (443) 269-1555 x361 Baltimore, Maryland 21218 -
- -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
