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 2/2] revision: implement --show-linear-break for --graph
Date: Mon, 18 Jan 2021 02:56:24 -0500 [thread overview]
Message-ID: <04c81462-3181-37d7-0109-4292040b84e9@pdinc.us> (raw)
In-Reply-To: <xmqq35yzknbr.fsf@gitster.c.googlers.com>
On 1/17/21 9:09 PM, Junio C Hamano wrote:
> Junio C Hamano<gitster@pobox.com> writes:
>
>> In other words, revs->break_revision_mark is left NULL unless
>> --show-linear-break is given.
>>
>>> @@ -4192,8 +4192,8 @@ const char *get_revision_mark(const struct rev_info *revs, const struct commit *
>>> else
>>> return ">";
>>> } else if (revs->graph) {
>>> - if (!commit->parents)
>>> - return "#";
>>> + if (revs->break_revision_mark && !commit->parents)
>>> + return revs->break_revision_mark;
>> And that causes this to break. Now "--graph" alone won't show '#'
>> for the root commits, despite that is what [1/2] wanted to do.
>>
>> Here is a fix-up, plus some minimum tests.
> Having said all that, I do not mind if the new markings were
> activated only when --show-linear-break option (or a separate new
> option) is given. But if that is where we want to go, your [1/2]
> that uses the new markings unconditionally is a regression.
>
> A better organization, if we wanted to have multiple and smaller
> steps than a single whole thing, would be:
>
> [1/2] Introduce a new "--mark-root-commits" option, or abuse the
> existing "--show-linear-break" option, and change "*<>"
> marking used for commits to "#LR" (or whatever appropriate)
> when the option is in effect. Document the behaviour and add
> tests.
>
> [2/2] Introduce "--show-linear-break=<custom-value>" option.
> Document the behaviour and add tests.
>
> If you apply [1/2] and [2/2] with the earlier fixes I sent, you'll
> see many fallouts from existing tests, as the representation of the
> root commit is changed unconditionally. We view breakages of tests
> as a rough estimate of how badly end-user scripts could break, and
> the picture was not very pretty. And that is why I am suggesting
> the above "only do the new markings when asked, not unconditionally"
> approach.
Sorry. I didn't make this clear. It is not an accident that patch 1
denotes root commits unconditionally and patch 2 makes it optional. I
present two choices. If we prefer to unconditionally denote root
commits, patch 2 may be left out, otherwise, patch 1 should be squashed
away.
I didn't have an opinion towards either option, but you make a good
point about end-user scripts.
> I still am skeptical that spending 3 more letters to denote roots is
> worth it, though.
Me too, but I think a user-defined mark needs to be a string to support
Unicode characters.
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
- -
- 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-18 7:57 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
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 [this message]
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=04c81462-3181-37d7-0109-4292040b84e9@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.