From: Kyle Marek <kmarek@pdinc.us>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jason Pyeron <jpyeron@pdinc.us>,
git@vger.kernel.org,
Philippe Blain <levraiphilippeblain@gmail.com>
Subject: Re: [PATCH 1/2] revision: Denote root commits with '#'
Date: Tue, 19 Jan 2021 22:25:48 -0500 [thread overview]
Message-ID: <460257a2-478a-eb4c-f6fa-b1cc55384cd5@pdinc.us> (raw)
In-Reply-To: <xmqq1reginnq.fsf@gitster.c.googlers.com>
On 1/19/21 5:10 PM, Junio C Hamano wrote:
> Kyle Marek <kmarek@pdinc.us> 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 -
- -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2021-01-20 3:27 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-14 18:30 add a blank line when a commit has no parent in log output? Jason Pyeron
2021-01-14 19:29 ` Philippe Blain
2021-01-14 20:44 ` Jason Pyeron
2021-01-17 11:03 ` [PATCH 0/2] Option to modify revision mark for root commits Kyle Marek
2021-01-17 11:03 ` [PATCH 1/2] revision: Denote root commits with '#' Kyle Marek
2021-01-17 21:10 ` Junio C Hamano
2021-01-18 7:56 ` Kyle Marek
2021-01-18 19:15 ` Junio C Hamano
2021-01-18 20:33 ` Junio C Hamano
2021-01-19 7:43 ` Kyle Marek
2021-01-19 22:10 ` Junio C Hamano
2021-01-20 3:25 ` Kyle Marek [this message]
2021-01-20 6:47 ` Junio C Hamano
2021-01-20 15:11 ` Jason Pyeron
2021-01-20 21:52 ` Junio C Hamano
2021-01-20 23:01 ` Jason Pyeron
2021-01-23 18:07 ` Junio C Hamano
2021-01-23 23:02 ` Jason Pyeron
2021-01-23 23:45 ` Junio C Hamano
2021-01-24 0:02 ` Jason Pyeron
2021-01-25 7:00 ` Junio C Hamano
2021-01-17 22:44 ` Junio C Hamano
2021-01-17 11:03 ` [PATCH 2/2] revision: implement --show-linear-break for --graph Kyle Marek
2021-01-17 22:56 ` Junio C Hamano
2021-01-18 2:09 ` Junio C Hamano
2021-01-18 7:56 ` Kyle Marek
2021-01-18 21:01 ` Junio C Hamano
2021-01-19 7:44 ` Kyle Marek
2021-01-15 1:12 ` add a blank line when a commit has no parent in log output? Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=460257a2-478a-eb4c-f6fa-b1cc55384cd5@pdinc.us \
--to=kmarek@pdinc.us \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jpyeron@pdinc.us \
--cc=levraiphilippeblain@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).