add a blank line when a commit has no parent in log output?

add a blank line when a commit has no parent in log output?

[Jason Pyeron]

Take this git log --format="%C(auto) %h% ad%d% s%C(green)% aE" --graph --date=short

| | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
| | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
| | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)

One might assume 5505e019c2 and 3e658f4085 are related. But git cat-file -p 5505e019c2
tree 546c6b71f01e7fd086c8adb832518240b71a9075
author sam swindell <xxxxxx@xxxx> 1404878701 -0400
committer sam swindell <xxxxxx@xxxx> 1404878701 -0400

initial


Is there a way to have it look like:

| | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
| | |
| | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
| | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)

Or 

| | | #  5505e019c2 2014-07-09 initial xxxxxx@xxxx
| | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
| | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)

Respectfully,

Jason Pyeron


--
Jason Pyeron  | Architect
PD Inc        |
10 w 24th St  |
Baltimore, MD |
 
.mil: jason.j.pyeron.ctr...
.com: jpyeron@pdinc.us
tel : 202-741-9397

Re: add a blank line when a commit has no parent in log output?

[Philippe Blain]

Hi Jason,

Le 2021-01-14 à 13:30, Jason Pyeron a écrit :
> Take this git log --format="%C(auto) %h% ad%d% s%C(green)% aE" --graph --date=short
> 
> | | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
> 
> One might assume 5505e019c2 and 3e658f4085 are related. But git cat-file -p 5505e019c2
> tree 546c6b71f01e7fd086c8adb832518240b71a9075
> author sam swindell <xxxxxx@xxxx> 1404878701 -0400
> committer sam swindell <xxxxxx@xxxx> 1404878701 -0400
> 
> initial
> 
> 
> Is there a way to have it look like:
> 
> | | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> | | |
> | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
> 
> Or
> 
> | | | #  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
> 

If you remove '--graph', then you can add '--show-linear-break' [1]. Unfortunately
these two options do not work together. I think your suggestion to have the '*'
be changed to '#' for root commit is a great idea.

In the mean time, I use this trick:

     git log --date=short --format='%C(auto) %h% [%<(2,trunc)%p] ad%d% s%C(green)% aE'

This adds the abbreviated parent hashes (%p) but truncated to 2 characters ([2], [3]). So
the brackets will be empty for root commits.

Cheers,

Philippe.


[1] https://git-scm.com/docs/git-log#Documentation/git-log.txt---show-linear-breakltbarriergt
[2] https://git-scm.com/docs/git-log#Documentation/git-log.txt-empem
[3] https://git-scm.com/docs/git-log#Documentation/git-log.txt-emltltNgttruncltruncmtruncem

RE: add a blank line when a commit has no parent in log output?

[Jason Pyeron]

Kyle:

Need you to whip up a patch (back port it to current Cygwin git too), see below. It will help with cleaning up Cresaptown branches. Or if you think Watson can do it, give it to him.

> -----Original Message-----
> From: Philippe Blain <levraiphilippeblain@gmail.com>
> Sent: Thursday, January 14, 2021 2:29 PM
> To: git@vger.kernel.org; Jason Pyeron <jpyeron@pdinc.us>
> Subject: Re: add a blank line when a commit has no parent in log output?
> 
> Hi Jason,
> 
> Le 2021-01-14 à 13:30, Jason Pyeron a écrit :
> > Take this git log --format="%C(auto) %h% ad%d% s%C(green)% aE" --graph --date=short
> >
> > | | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> > | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> > | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
> >
> > One might assume 5505e019c2 and 3e658f4085 are related. But git cat-file -p 5505e019c2
> > tree 546c6b71f01e7fd086c8adb832518240b71a9075
> > author sam swindell <xxxxxx@xxxx> 1404878701 -0400
> > committer sam swindell <xxxxxx@xxxx> 1404878701 -0400
> >
> > initial
> >
> >
> > Is there a way to have it look like:
> >
> > | | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> > | | |
> > | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> > | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
> >
> > Or
> >
> > | | | #  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> > | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> > | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
> >
> 
> If you remove '--graph', then you can add '--show-linear-break' [1]. Unfortunately
> these two options do not work together. I think your suggestion to have the '*'
> be changed to '#' for root commit is a great idea.

Patch description

When --graph is used

--show-linear-break converts the * to a #

--show-linear-break=x converts the * to a x

> 
> In the mean time, I use this trick:
> 
>      git log --date=short --format='%C(auto) %h% [%<(2,trunc)%p] ad%d% s%C(green)% aE'
> 
> This adds the abbreviated parent hashes (%p) but truncated to 2 characters ([2], [3]). So
> the brackets will be empty for root commits.
> 
> Cheers,
> 
> Philippe.
> 
> 
> [1] https://git-scm.com/docs/git-log#Documentation/git-log.txt---show-linear-breakltbarriergt
> [2] https://git-scm.com/docs/git-log#Documentation/git-log.txt-empem
> [3] https://git-scm.com/docs/git-log#Documentation/git-log.txt-emltltNgttruncltruncmtruncem

[PATCH 0/2] Option to modify revision mark for root commits

[Kyle Marek]

This patch series allows --show-linear-break to be used with --graph,
allowing for the revision mark to be changed for root commits.

Feel free to squash away PATCH 1, or maybe even discard PATCH 2.

Jason: tested against Cygwin x86_64/release/git/git-2.30.0-1-src.tar.xz

Note: PATCH 2 revision.c:2410 makes a second copy of optarg. This may
not be necessary.

Background:

The use case is --graph --oneline with unrelated histories. For example,
in a hypothetical repository with an orphaned "prebuilt" branch
containing builds of the master branch, the history may look like:

kmarek@kyle-ppc64le /tmp/somerepo
$ git log --graph --all --oneline
* 02190b6 (prebuilt) add aarch64
* 7b873f6 add x86_64
* 26cc783 add ppc64le
* 5b7186e (HEAD -> master) add Makefile
* ea69093 implement cmdline parsing
* a65df8a add main.c
* 7727eb3 Initial commit

At first sight, the above log implies that 26cc783's parent is 5b7186e,
or that master is an ancestor to prebuilt, but 26cc783 is the start of a
new history:

kmarek@kyle-ppc64le /tmp/somerepo
$ git log --graph --oneline master
* 5b7186e (HEAD -> master) add Makefile
* ea69093 implement cmdline parsing
* a65df8a add main.c
* 7727eb3 Initial commit

kmarek@kyle-ppc64le /tmp/somerepo
$ git log --graph --oneline prebuilt
* 02190b6 (prebuilt) add aarch64
* 7b873f6 add x86_64
* 26cc783 add ppc64le

To identify the start of a new history:

kmarek@kyle-ppc64le /tmp/somerepo
$ git log --graph --all --oneline --show-linear-break
* 02190b6 (prebuilt) add aarch64
* 7b873f6 add x86_64
# 26cc783 add ppc64le
* 5b7186e (HEAD -> master) add Makefile
* ea69093 implement cmdline parsing
* a65df8a add main.c
# 7727eb3 Initial commit

kmarek@kyle-ppc64le /tmp/somerepo
$ git log --graph --all --oneline --show-linear-break=I
* 02190b6 (prebuilt) add aarch64
* 7b873f6 add x86_64
I 26cc783 add ppc64le
* 5b7186e (HEAD -> master) add Makefile
* ea69093 implement cmdline parsing
* a65df8a add main.c
I 7727eb3 Initial commit

Kyle Marek (2):
  revision: Denote root commits with '#'
  revision: implement --show-linear-break for --graph

 Documentation/rev-list-options.txt |  7 +++++++
 log-tree.c                         |  2 +-
 revision.c                         | 10 ++++++----
 revision.h                         |  1 +
 4 files changed, 15 insertions(+), 5 deletions(-)

-- 
2.29.2

[PATCH 1/2] revision: Denote root commits with '#'

[Kyle Marek]

This aids in identifying where an unrelated branch history starts when
using `git log --graph --oneline --all`

Signed-off-by: Kyle Marek <kmarek@pdinc.us>
---
 revision.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 9dff845bed..8556923de8 100644
--- a/revision.c
+++ b/revision.c
@@ -4191,9 +4191,11 @@ const char *get_revision_mark(const struct rev_info *revs, const struct commit *
 			return "<";
 		else
 			return ">";
-	} else if (revs->graph)
+	} else if (revs->graph) {
+		if (!commit->parents)
+			return "#";
 		return "*";
-	else if (revs->cherry_mark)
+	} else if (revs->cherry_mark)
 		return "+";
 	return "";
 }
-- 
2.29.2

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

Kyle Marek <kmarek@pdinc.us> writes:

> This aids in identifying where an unrelated branch history starts when
> using `git log --graph --oneline --all`
>
> Signed-off-by: Kyle Marek <kmarek@pdinc.us>
> ---
>  revision.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

No tests?

> diff --git a/revision.c b/revision.c
> index 9dff845bed..8556923de8 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -4191,9 +4191,11 @@ const char *get_revision_mark(const struct rev_info *revs, const struct commit *
>  			return "<";
>  		else
>  			return ">";
> -	} else if (revs->graph)
> +	} else if (revs->graph) {
> +		if (!commit->parents)
> +			return "#";
>  		return "*";
> -	else if (revs->cherry_mark)
> +	} else if (revs->cherry_mark)
>  		return "+";
>  	return "";
>  }

Here is what I tried to come up with, but somehow the "#" marker is
not showing for me.

The "counted plus --left-right" tests stress why a single "#" is not
good enough.  I think the patch also needs to replace "<" and ">"
for root commits that are left and right---in the tests, I used "L"
to denote "root that is on the left side" (and "R" for the right
side) instead of single "#", so that we do not to lose information.

By the way, as I already said in the original thread, I do not think
the '#' marking is a good idea; I'd rather see the root commit shown
by shifting columns.

Anyway, here is to test [1/2].

 t/t6020-rev-list-boundary.sh | 132 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git i/t/t6020-rev-list-boundary.sh w/t/t6020-rev-list-boundary.sh
new file mode 100755
index 0000000000..f25e041951
--- /dev/null
+++ w/t/t6020-rev-list-boundary.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+
+test_description='rev-list/log boundary and root'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit A &&
+	test_commit B &&
+	git reset --hard A &&
+	test_commit C &&
+
+	git checkout --orphan side &&
+	git rm -fr . &&
+	test_commit X &&
+	test_commit Y &&
+
+	test_tick && git merge --allow-unrelated-histories -m "M" B &&
+	test_tick && git merge -m "N" C &&
+	test_commit Z
+'
+
+test_expect_success 'log with boundary' '
+	git log --graph --boundary --format='%s' ^A ^X Z >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	* Z
+	*   N
+	|\  Q
+	| * C
+	* |   M
+	|\ \  Q
+	| * | B
+	| |/  Q
+	* | Y
+	o | X
+	 /  Q
+	o A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'log --left-right with symmetric boundary' '
+	git log --graph --left-right --boundary --format='%s' B...C >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	> C
+	| < B
+	|/  Q
+	o A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'log --left-right with asymmetric boundary' '
+	git log --graph --left-right --boundary --format='%s' ^A ^X Z >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	> Z
+	>   N
+	|\  Q
+	| > C
+	> |   M
+	|\ \  Q
+	| > | B
+	| |/  Q
+	> | Y
+	o | X
+	 /  Q
+	o A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_failure 'log down to root' '
+	git log --graph --format='%s' Z >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	* Z
+	*   N
+	|\  Q
+	| * C
+	* |   M
+	|\ \  Q
+	| * | B
+	| |/  Q
+	| # A
+	* Y
+	# X
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_failure 'log down to root' '
+	git log --graph --format='%s' B Y >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	* Y
+	# X
+	* B
+	# A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_failure 'log that happens to show root' '
+	git log --graph -3 --format='%s' B Y >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	* Y
+	# X
+	* B
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_failure 'log --left-right down to root' '
+	git log --graph --left-right --format='%s' B...Y >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	> Y
+	R X
+	< B
+	L A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_failure 'log --left-right that happens to show root' '
+	git log --graph -3 --left-right --format='%s' B...Y >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	> Y
+	R X
+	< B
+	EOF
+	test_cmp expect actual
+'
+
+test_done

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Kyle Marek]

On 1/17/21 4:10 PM, Junio C Hamano wrote:
> Kyle Marek<kmarek@pdinc.us>  writes:
>
>> This aids in identifying where an unrelated branch history starts when
>> using `git log --graph --oneline --all`
>>
>> Signed-off-by: Kyle Marek<kmarek@pdinc.us>
>> ---
>>   revision.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
> No tests?

I'm not very familiar with the code base. I now see the t/README file.

>> diff --git a/revision.c b/revision.c
>> index 9dff845bed..8556923de8 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -4191,9 +4191,11 @@ const char *get_revision_mark(const struct rev_info *revs, const struct commit *
>>   			return "<";
>>   		else
>>   			return ">";
>> -	} else if (revs->graph)
>> +	} else if (revs->graph) {
>> +		if (!commit->parents)
>> +			return "#";
>>   		return "*";
>> -	else if (revs->cherry_mark)
>> +	} else if (revs->cherry_mark)
>>   		return "+";
>>   	return "";
>>   }
> Here is what I tried to come up with, but somehow the "#" marker is
> not showing for me.
>
> The "counted plus --left-right" tests stress why a single "#" is not
> good enough.  I think the patch also needs to replace "<" and ">"
> for root commits that are left and right---in the tests, I used "L"
> to denote "root that is on the left side" (and "R" for the right
> side) instead of single "#", so that we do not to lose information.
>
> By the way, as I already said in the original thread, I do not think
> the '#' marking is a good idea; I'd rather see the root commit shown
> by shifting columns.

Sorry, I wasn't subscribed to the list until Jason CC'd me on his 
request. I also wasn't aware of --left-right.

I'll investigate the revision-mark shifting idea. I am concerned that it 
would get complicated if a graph edge extends around a revision that 
needs to be shifted, but I'm finding it difficult to produce this with 
--graph:

*   8d82d0a (HEAD -> master) Merge branch 'o1'
|\
| * 3479914 (o1) O1
| * a674e07 O1        <-- root commit
| * 2237b52 (t) T
| * f525fa5 T
|/
* f15f936 A
| * 9e289ed (u) U
|/
* ee911c8 initial     <-- root commit

vs:

*   8ee9b14 (HEAD -> master) Merge branch 'u'
|\
| * ed1990f (u) U
* |   277f31c Merge branch 'o1'
|\ \
| * | eaa71bb (o1) O1
| * | 9203a43 O1      <-- root commit
|  /
| | * bc2c4d9 (t) T
| | * 2d3c03b T
| |/
|/|
* | 6a26183 A
|/
* da85ccf initial     <-- root commit
  

Thoughts? Will git ever graph something like:

*
|\
| *
* |
|\ \
| * | <-- root commit
| * | <-- some head
|/ /
* /
|/
*     <-- root commit

-- 

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-                                                               -
- Kyle Marek                        PD Inc.http://www.pdinc.us  -
- Jr. Developer                     10 West 24th Street #100    -
- +1 (443) 269-1555 x361            Baltimore, Maryland 21218   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

Kyle Marek <kmarek@pdinc.us> writes:

> I'll investigate the revision-mark shifting idea. I am concerned that
> it would get complicated if a graph edge extends around a revision
> that needs to be shifted,...

The "current graph layout makes it harder to see where the root is"
problem has a natural solution: fix the graph layout so that the
root is easily visible.

I however think it is a much harder approach to solve than using a
different mark for root commits, and it is the reason why there have
been at least a few attempts in the past that did essentially the
same patch as yours, plus the "linear break" which we accepted.

> *   8d82d0a (HEAD -> master) Merge branch 'o1'
> |\
> | * 3479914 (o1) O1
> | * a674e07 O1        <-- root commit
> | * 2237b52 (t) T
> | * f525fa5 T
> |/
> * f15f936 A
> | * 9e289ed (u) U
> |/
> * ee911c8 initial     <-- root commit
>
> vs:
>
> *   8ee9b14 (HEAD -> master) Merge branch 'u'
> |\
> | * ed1990f (u) U
> * |   277f31c Merge branch 'o1'
> |\ \
> | * | eaa71bb (o1) O1
> | * | 9203a43 O1      <-- root commit
> |  /
> | | * bc2c4d9 (t) T
> | | * 2d3c03b T
> | |/
> |/|
> * | 6a26183 A
> |/
> * da85ccf initial     <-- root commit

Sorry, I am not quite sure what you are trying to illustrate with
the comparison between the above two.  The latter makes it clear
that 9203a43 and da85ccf do not have parents in the depicted part of
the history [*1*].

In the former one, does 2237b52 have no child in the depicted part of
the history, and is the problem that it appears as if it has a674e07
as a child?  I wonder if we can just shift them, either:

> *   8d82d0a (HEAD -> master) Merge branch 'o1'
> |\__
> |   * 3479914 (o1) O1
> |   * a674e07 O1        <-- root commit
> | * 2237b52 (t) T
> | * f525fa5 T
> |/
> * f15f936 A

or

> *   8d82d0a (HEAD -> master) Merge branch 'o1'
> |\
> | * 3479914 (o1) O1
> | * a674e07 O1        <-- root commit
> |   * 2237b52 (t) T
> | __* f525fa5 T
> |/
> * f15f936 A

Or we could punt to show it with an extra blank line, although it is
suboptimial.

> *   8d82d0a (HEAD -> master) Merge branch 'o1'
> |\
> | * 3479914 (o1) O1
> | * a674e07 O1        <-- root commit
> |
> | * 2237b52 (t) T
> | * f525fa5 T
> |/
> * f15f936 A


[Footnote]

*1* Stepping back a bit, I think concentrating too much on "is it
    root?" is a wrong way to think about the problem.  Suppose you
    have two histories, e.g. (time flows from left to right; A and X
    are roots)

            A---B
                 \
          X---Y---Z

    and doing "git log --graph --oneline Z" would show A, B, X, Y
    and Z.

    If it benefits to show "A" (and "X") specially in the graph,
    that would mean that the current algorithm would show some other
    commit after showing A (probably X if it goes in chronological
    order), and it probably is confusing because X is shown on the
    same column as A, when there is no parent-child relationship
    between them (A is root after all).

    We are trying to highlight that A is not a child of anybody by
    using '#' instead.

    But in a slightly modified graph:

          C
         /
        O---A---B
                 \
          X---Y---Z

    if you do "git log --graph --oneline C..Z", you should see the
    same commits listed as above (A, B, X, Y and Z), and most likely
    in the same order.

    So special casing by "Ah, A is a root commit, so let's show it
    with '#'" does not help, even though we are facing exactly the
    same problem in the latter graph.

    And the right way to look at it is "does A have any parent in
    the part of the history being shown?", not "does A have any
    parent?"  Then 'A' will get exactly the same treatment in the
    two examples, and the visual problem that makes A appear as if
    it has parent-child relationship with unrelated commit X goes
    away.

    

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> [Footnote]
>
> *1* Stepping back a bit, I think concentrating too much on "is it
>     root?" is a wrong way to think about the problem.  Suppose you
>     have two histories, e.g. (time flows from left to right; A and X
>     are roots)

A shorter and more concrete example.  Start from an empty repository:

	$ git init
	$ git commit --allow-empty -m Aroot
	$ git checkout --orphan side
	$ git commit --allow-empty -m Xroot
	$ git log --all --graph --oneline
        * a1f7cb2 (HEAD -> side) Xroot
        * b6fb655 (master) Aroot

These depict two root commits, Aroot and Xroot, and no other
commits.  We do want to show that these two commits do not have
parent-child relationship at all, and your (and a few proposals made
by other in the past) solution was to show them both with "#".

Continuing in the same repository:

	$ git checkout --orphan another
	$ git commit --allow-empty -m Oroot
	$ git commit --allow-empty -m A
	$ git log --graph --oneline ^another^ another side
        * eddf116 (HEAD -> another) A
        * a1f7cb2 (side) Xroot

These depict two commits, A and Xroot, and no other commits.  We
also want to show that these two commits do not have parent-child
relationship at all, but if we paint Xroot with "#", it still makes
it appear that A is a child of Xroot.

>     And the right way to look at it is "does A have any parent in
>     the part of the history being shown?", not "does A have any
>     parent?"  Then 'A' will get exactly the same treatment in the
>     two examples, and the visual problem that makes A appear as if
>     it has parent-child relationship with unrelated commit X goes
>     away.

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

	int no_interesting_parents(struct commit *commit)
	{
		struct commit_list *parents = commit->parents;

		while (parents) {
			if (!(parents->object.flags & UNINTERESTING))
				return 0;
			parents = parents->next;
		}
		return 1;
	}

or something like that should serve as a replacement, i.e.

	return !commit->parents ? "#" : "*";

would become

	return no_interesting_parents(commit) ? "#" : "*";

Hmm?

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Kyle Marek]

On 1/18/21 3:33 PM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> [Footnote]
>>
>> *1* Stepping back a bit, I think concentrating too much on "is it
>>      root?" is a wrong way to think about the problem.  Suppose you
>>      have two histories, e.g. (time flows from left to right; A and X
>>      are roots)
> A shorter and more concrete example.  Start from an empty repository:
>
> 	$ git init
> 	$ git commit --allow-empty -m Aroot
> 	$ git checkout --orphan side
> 	$ git commit --allow-empty -m Xroot
> 	$ git log --all --graph --oneline
>          * a1f7cb2 (HEAD -> side) Xroot
>          * b6fb655 (master) Aroot
>
> These depict two root commits, Aroot and Xroot, and no other
> commits.  We do want to show that these two commits do not have
> parent-child relationship at all, and your (and a few proposals made
> by other in the past) solution was to show them both with "#".
>
> Continuing in the same repository:
>
> 	$ git checkout --orphan another
> 	$ git commit --allow-empty -m Oroot
> 	$ git commit --allow-empty -m A
> 	$ git log --graph --oneline ^another^ another side
>          * eddf116 (HEAD -> another) A
>          * a1f7cb2 (side) Xroot
>
> These depict two commits, A and Xroot, and no other commits.  We
> also want to show that these two commits do not have parent-child
> relationship at all, but if we paint Xroot with "#", it still makes
> it appear that A is a child of Xroot.
>
>>      And the right way to look at it is "does A have any parent in
>>      the part of the history being shown?", not "does A have any
>>      parent?"  Then 'A' will get exactly the same treatment in the
>>      two examples, and the visual problem that makes A appear as if
>>      it has parent-child relationship with unrelated commit X goes
>>      away.
> 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.  Perhaps
>
> 	int no_interesting_parents(struct commit *commit)
> 	{
> 		struct commit_list *parents = commit->parents;
>
> 		while (parents) {
> 			if (!(parents->object.flags & UNINTERESTING))
> 				return 0;
> 			parents = parents->next;
> 		}
> 		return 1;
> 	}
>
> or something like that should serve as a replacement, i.e.
>
> 	return !commit->parents ? "#" : "*";
>
> would become
>
> 	return no_interesting_parents(commit) ? "#" : "*";
>
> Hmm?

Okay, I see what you mean. Fixing --graph to avoid implying ancestry 
sounds like a better approach to me.

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, too.

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?

If so, what would you like to see out of the --left-right issue? Maybe 
"--mark-left-root=<mark>" and "--mark-right-root=<mark>", so multi-byte 
strings may be used? Can there be more than one root on either side? (so 
the option would use a plural "roots" instead of "root"?)

-- 

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-                                                               -
- Kyle Marek                        PD Inc. http://www.pdinc.us -
- Jr. Developer                     10 West 24th Street #100    -
- +1 (443) 269-1555 x361            Baltimore, Maryland 21218   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

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

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.

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

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 am hoping that a single option can give three-tuple that replaces
the usual "<*>", with perhaps the default of "{#}" or something.

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.


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

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Kyle Marek]

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   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

Kyle Marek <kmarek@pdinc.us> writes:

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

I actually do not see that as a problem.  In the past several years,
I've never needed to see "log --graph" output that goes all the way
down to the roots, unless I was playing with a toy repository in
order to tweak and/or develop a feature in Git that draws the graph.

Besides, such root commtis in real life projects would not say "X",
but something along the lines of "my very initial commit", which
would be much more "/<search>" friendly to pagers than "#".

So, no, sorry, but I do not buy "root is more special" at all.

Thanks.

RE: [PATCH 1/2] revision: Denote root commits with '#'

[Jason Pyeron]

> -----Original Message-----
> From: Junio C Hamano
> Sent: Wednesday, January 20, 2021 1:48 AM
> 
> Kyle Marek writes:
> 
> > 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.
> 
> I actually do not see that as a problem.  In the past several years,
> I've never needed to see "log --graph" output that goes all the way

I respect your needs, but they conflict with others' needs, while this enhancement to resolve an ambiguity does not impede your needs and solves others' needs. Please do not impose your exclusive use cases upon everyone.

> down to the roots, unless I was playing with a toy repository in

I brought this issue up because several repositories in use have this issue. Two repositories immediately at hand have 35k+ and 2500+ commits each. These are repositories used by professionals and contain actual source code. ( I know your "toy repository" tone was not meant as an insult because I read your emails daily, Kyle may not have )

> order to tweak and/or develop a feature in Git that draws the graph.
> 
> Besides, such root commtis in real life projects would not say "X",
> but something along the lines of "my very initial commit", which

Here is where a fundamental (feature) issue of git rears its ugly head. You cannot fix the commit meta data (e.g. message) after the fact. Humans write the message, and it does not always write a message the is easily recognizable as such, no less easy to search.

> would be much more "/<search>" friendly to pagers than "#".

Here are some messages:

bug 2252 test case (e.g. for tomcat 9 with unpackWARs=false)
Add migrate-from-blackfat.sql
Initial commit from Create React App
parrent pom
initial commit
Base applet
intial
Initial commit
initial
import prod 
import prod sql 
import prod 
import coop/dev 
import prod CMIS.zip


Here we have commits without the word initial, initial misspelled, or in different case.

Let's not bike shed this issue. The left/right issues are a great catch from a peer review point of view.

I'll ask the following questions, besides the left right and test case issues:

What quality issues exists with the patch (e.g. bugs, strategy, etc)?

How can the proposed additional features be captured for future implementation?

Do we want to continue discussion on option naming?

Are there other questions to discuss?

Respectfully,

Jason Pyeron

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

"Jason Pyeron" <jpyeron@pdinc.us> writes:

>> I actually do not see that as a problem.  In the past several years,
>> I've never needed to see "log --graph" output that goes all the way
>
> I respect your needs, but they conflict with others' needs, while
> this enhancement to resolve an ambiguity does not impede your
> needs and solves others' needs.

I am questioning if such "needs" really exist in the first place.

Among 35k+ commits in the example project, if you had more than a
few dozens of roots, then it may make sense to highlight them
differently from ordinary commits whether they have parents in the
shown part of the history.  It's like "log --decorate" shows branch
tips marked specially.

Yes, I am saying that such a "this is root" marking, if it is
valuable, should go on a part of "log --oneline" output that is
shown even without "--graph", just like we annotate the commit with
"(branch name)" in the output, instead of painting the commit in the
graph by replacing the '*' node with something else.

And how often do you really need to see commits near the root, say
the earliest 100 commits, in the 35k+ commit history?  Is it really
necessary to tell which among these 100 is the root?  What problem
does it solve?  Perhaps I am reacting to your solution without
seeing the problem you are trying to solve?  First, I took the
"replace <*> with {#}" as a solution for "parenthood becomes unclear
in the --graph output" problem, and pointed out that the solution
for that issue should apply to not just root commits but equally to
the ones above the boundary.

But it seems that I am hearing that it is not "graph showing false
parenthood" problem that you were trying to solve, but "I want to
see root differently for unspecified reason".

I am asking why, and if the reason is because there are nontrivial
number of them sprinkled throughout the history, I am offering my
opinion that something like how we show the commits at the tips of
branches and tagged ones would be a better model than changing the
letter used for the node in the graph.

> Here are some messages:
>
> bug 2252 test case (e.g. for tomcat 9 with unpackWARs=false)
> Add migrate-from-blackfat.sql
> Initial commit from Create React App
> parrent pom
> initial commit
> Base applet
> intial
> Initial commit
> initial
> import prod 
> import prod sql 
> import prod 
> import coop/dev 
> import prod CMIS.zip

You seem to have problems with not just root commits ;-)
How many of these 5 "initial" commits are root?

> I'll ask the following questions, besides the left right and test case issues:
>
> What quality issues exists with the patch (e.g. bugs, strategy, etc)?

By strategy I take that you mean design.  We've been talking about
it, right?  Until that gets more or less settled, line-by-line bug
hunting tends to become a waste of time, and I haven't had a chance
to afford extra review bandwidth to dedicate to this topic.

Now the problem being solved seems to be changing, so I am not sure
how close to be "done" the posted patch is to the real solution.
Sorry.

RE: [PATCH 1/2] revision: Denote root commits with '#'

[Jason Pyeron]

Summary: --graph used with --oneline sometimes produces ambiguous output when more than one commit has no parents and are not yet merged

> From: Junio C Hamano
> Sent: Wednesday, January 20, 2021 4:52 PM
> 
> "Jason Pyeron" writes:
> 
> >> I actually do not see that as a problem.  In the past several years,
> >> I've never needed to see "log --graph" output that goes all the way
> >
> > I respect your needs, but they conflict with others' needs, while
> > this enhancement to resolve an ambiguity does not impede your
> > needs and solves others' needs.
> 
> I am questioning if such "needs" really exist in the first place.
> 
> Among 35k+ commits in the example project, if you had more than a
> few dozens of roots, then it may make sense to highlight them
> differently from ordinary commits whether they have parents in the
> shown part of the history.  It's like "log --decorate" shows branch
> tips marked specially.

That could work too.

> 
> Yes, I am saying that such a "this is root" marking, if it is
> valuable, should go on a part of "log --oneline" output that is
> shown even without "--graph", just like we annotate the commit with

I do not have any preferences beyond not "being lied to by git graph".

| * 22222
| * 11111
| * 33333
| * 44444

Implies that 11111 and 33333 have a parent / child relationship.

Quoting the man page, "--graph Draw a text-based graphical representation of the commit history on the left hand side of the output. This may cause extra lines to be printed in between commits, in order for the graph history to be drawn properly", would be preferable to add blank lines.

> "(branch name)" in the output, instead of painting the commit in the
> graph by replacing the '*' node with something else.
> 
> And how often do you really need to see commits near the root, say
> the earliest 100 commits, in the 35k+ commit history?  Is it really
> necessary to tell which among these 100 is the root?  

Yes, and the assumption that they are at the beginning is flawed too.

$ git log --oneline --graph --all | cat -n | egrep $(git rev-list --max-parents=0 --all | cut -c 1-8 | tr '\n' '|' | head -c -1)
    87  | | * be2c70b7 bug 2252 test case (e.g. for tomcat 9 with unpackWARs=false)
  2161  | | * 8ef73128 Add migrate-from-blackfat.sql
  2164  | | * 5505e019 initial
  2235  | | | | | | | | | | | | | * 83337c67 intial
  2921  | | | | * ca14dc49 Initial commit
  2931  | | | * cbdce824 initial commit
  2963  | | * 8f1828c1 Base applet
  2971  | * 658af21f parrent pom
  3026  * 8356af31 Initial commit from Create React App

git log --oneline --graph produces 3026 lines in this example.

> What problem does it solve?  

Avoiding confusion and non-compliance with the man page, which wastes human's time.

> Perhaps I am reacting to your solution without
> seeing the problem you are trying to solve?  First, I took the
> "replace <*> with {#}" as a solution for "parenthood becomes unclear
> in the --graph output" problem, and pointed out that the solution
> for that issue should apply to not just root commits but equally to
> the ones above the boundary.
> 

I have no objection to that either as it neither helps or hinders the solution to the real and initial issue.

> But it seems that I am hearing that it is not "graph showing false
> parenthood" problem that you were trying to solve, 

It is that graph is implying false parenthood. There was no intention for that (only) issue to morph.

> but "I want to
> see root differently for unspecified reason".

There is only one reason, the same reason that prompted the original email. Adjacent commits in the --graph formatted output were connected when they are actually not connected.

To quote earlier > From: Junio C Hamano
> Sent: Thursday, January 14, 2021 8:12 PM
> > | | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> > | | |
> > | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> > | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
> >
> ... is not so great in that it wastes a line, and the break
> won't be as noticeable when --graph is *not* used with --oneline.

No, because there would be no line connecting it.

| | | Date:   Tue Sep
| | |
| | |     Added defau
| | |
| | * commit 5505e019
| |   Author: xxxx
| |   Date:   Wed Jul
| |
| |       initial
| |
| | * commit 3e658f40
| | | Author: xxxx
| | | Date:   Tue Sep
| | |
| | |     Added defau
| | |
| | * commit ad148aaf
| | | Author: xxxx
| | | Date:   Tue Sep
| | |
| | |     Added defau
| | |

And to quote earlier > From: Junio C Hamano
> Sent: Thursday, January 14, 2021 8:12 PM
> > | | | #  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> > | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> > | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
> This latter variant won't work.  Imagine we are showing --left-right
> for example.  Which side does '#' belong to?

There was no concerns or aversions about left/right. It was later clarified that being able to use the pagers search would be nice.

> 
> I am asking why, and if the reason is because there are nontrivial
> number of them sprinkled throughout the history, I am offering my
> opinion that something like how we show the commits at the tips of
> branches and tagged ones would be a better model than changing the
> letter used for the node in the graph.

Happy to take that solution too, but does it fix the bug in the graph when used with --oneline? And don’t misunderstand me, this is a bug in --graph with --oneline.

> 
> > Here are some messages:
> >
> > bug 2252 test case (e.g. for tomcat 9 with unpackWARs=false)
> > Add migrate-from-blackfat.sql
> > Initial commit from Create React App
> > parrent pom
> > initial commit
> > Base applet
> > intial
> > Initial commit
> > initial
> > import prod
> > import prod sql
> > import prod
> > import coop/dev
> > import prod CMIS.zip
> 
> You seem to have problems with not just root commits ;-)
> How many of these 5 "initial" commits are root?

100%, it was from:

git log --oneline $(git rev-list --max-parents=0 --all) | cut -c 10-

> 
> > I'll ask the following questions, besides the left right and test case issues:
> >
> > What quality issues exists with the patch (e.g. bugs, strategy, etc)?
> 
> By strategy I take that you mean design.  We've been talking about
> it, right?  Until that gets more or less settled, line-by-line bug
> hunting tends to become a waste of time, and I haven't had a chance
> to afford extra review bandwidth to dedicate to this topic.
> 
> Now the problem being solved seems to be changing, so I am not sure
> how close to be "done" the posted patch is to the real solution.
> Sorry.

There was no intention for change, but adjustments were made based on feedback. For example, quoting earlier > From: Junio C Hamano
> Sent: Thursday, January 14, 2021 8:12 PM
> It would be great to show it more like this:
> 
>  | | |   * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
>  | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
>  | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
> 

> From: Junio C Hamano
> Sent: Tuesday, January 19, 2021 5:10 PM
> 
> I do not mind if the graph rendering fix does not happen yet again;
> IIRC the past contributors couldn't implement it, either.

This was a good idea, but not readily feasible.

So to close the loop, I would love to support the creation and integration of a patch to ensure "graph history s/to be/is/ drawn properly" and not lying to the reader of the graph about the ancestry.

And thank you for spending time on this thread, I think we can find a feasible and usable solution.

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

"Jason Pyeron" <jpyeron@pdinc.us> writes:

> Summary: --graph used with --oneline sometimes produces ambiguous
> output when more than one commit has no parents and are not yet
> merged
> ...
>> "(branch name)" in the output, instead of painting the commit in the
>> graph by replacing the '*' node with something else.
>> 
>> And how often do you really need to see commits near the root, say
>> the earliest 100 commits, in the 35k+ commit history?  Is it really
>> necessary to tell which among these 100 is the root?  
>
> Yes, and the assumption that they are at the beginning is flawed too.
>
> $ git log --oneline --graph --all | cat -n | egrep $(git rev-list --max-parents=0 --all | cut -c 1-8 | tr '\n' '|' | head -c -1)
>     87  | | * be2c70b7 bug 2252 test case (e.g. for tomcat 9 with unpackWARs=false)
>   2161  | | * 8ef73128 Add migrate-from-blackfat.sql
>   2164  | | * 5505e019 initial
>   2235  | | | | | | | | | | | | | * 83337c67 intial
>   2921  | | | | * ca14dc49 Initial commit
>   2931  | | | * cbdce824 initial commit
>   2963  | | * 8f1828c1 Base applet
>   2971  | * 658af21f parrent pom
>   3026  * 8356af31 Initial commit from Create React App
>
> git log --oneline --graph produces 3026 lines in this example.

Hmph.  Are you saying that you have 3000+ root commits in the 35k+
history?

Whether we add '[root]' decoration to the true roots (like
'(branchname)' decoration we add to branch tips), or painted '*' in
a different color (like '#'), you do not have to look for 'initial',
so having that many roots will not be a problem per-se with respect
to the "log" output, but there must be something strange going on.

I am not going to ask you why you need so many roots, because I
suspect that I will regret asking ;-).

By the way, I sense that your problem description is flip-flopping
again and I can no longer keep track of.  The way I read the message
I got from Kyle was, even when a graph has two commits that have no
parents in the visible part of the history, either Kyle wanted (or
Kyle got an impression after talking to you that you wanted) to see
these differently if one of them is a root and the other is non-root
(but happens to have none of its parents shown due to A..B range).
And that is why I started asking how meaningful to special case only
"root".

Now the message from you I am responding to in the "Sumary" above
says that it is not "root" but is about the placement of graph
nodes.

So, I dunno, with changing the description of the goalpost.  Now it
is that "root" is so not special at all and we only care about that
the a commit, none of whose parents are in the part of the shown
history, is shown in such a way that the user can tell that any
unrelated commits shown in the graph near it are not parents of such
a commit?  Or do you still want to show such a commit in two ways,
one for root and one for the ones above the boundary?

RE: [PATCH 1/2] revision: Denote root commits with '#'

[Jason Pyeron]

> -----Original Message-----
> From: Junio C Hamano <gitster@pobox.com>
> Sent: Saturday, January 23, 2021 1:07 PM
> 
> "Jason Pyeron" writes:
> 
> > Summary: --graph used with --oneline sometimes produces ambiguous
> > output when more than one commit has no parents and are not yet
> > merged
> > ...
> >> "(branch name)" in the output, instead of painting the commit in the
> >> graph by replacing the '*' node with something else.
> >>
> >> And how often do you really need to see commits near the root, say
> >> the earliest 100 commits, in the 35k+ commit history?  Is it really
> >> necessary to tell which among these 100 is the root?
> >
> > Yes, and the assumption that they are at the beginning is flawed too.
> >
> > $ git log --oneline --graph --all | cat -n | egrep $(git rev-list --max-parents=0 --all | cut -c 1-8
> | tr '\n' '|' | head -c -1)
> >     87  | | * be2c70b7 bug 2252 test case (e.g. for tomcat 9 with unpackWARs=false)
> >   2161  | | * 8ef73128 Add migrate-from-blackfat.sql
> >   2164  | | * 5505e019 initial
> >   2235  | | | | | | | | | | | | | * 83337c67 intial
> >   2921  | | | | * ca14dc49 Initial commit
> >   2931  | | | * cbdce824 initial commit
> >   2963  | | * 8f1828c1 Base applet
> >   2971  | * 658af21f parrent pom
> >   3026  * 8356af31 Initial commit from Create React App
> >
> > git log --oneline --graph produces 3026 lines in this example.
> 
> Hmph.  Are you saying that you have 3000+ root commits in the 35k+
> history?
> 

I think you misread the specific example of 9 roots in 3026 commits, distributed throughout history.

> Whether we add '[root]' decoration to the true roots (like
> '(branchname)' decoration we add to branch tips), or painted '*' in
> a different color (like '#'), you do not have to look for 'initial',
> so having that many roots will not be a problem per-se with respect
> to the "log" output, but there must be something strange going on.
> 
> I am not going to ask you why you need so many roots, because I
> suspect that I will regret asking ;-).
> 
> By the way, I sense that your problem description is flip-flopping
> again and I can no longer keep track of.  The way I read the message
> I got from Kyle was, even when a graph has two commits that have no
> parents in the visible part of the history, either Kyle wanted (or
> Kyle got an impression after talking to you that you wanted) to see
> these differently if one of them is a root and the other is non-root
> (but happens to have none of its parents shown due to A..B range).
> And that is why I started asking how meaningful to special case only
> "root".
> 

I may be having trouble with my writing, apologies.

Here is the issue (bug):

1. I never want to see a commit implied to be the parent of an unrelated commit.
2. I never want to see a commit implied to be the child of an unrelated commit.

--graph --oneline is broken with regards to the man page and my desire to not be confused by the implication of relationship for inappropriately connected nodes on the graph.

| | * 1234567 commit child of 2345678
| | * 2345678 the first commit, having no parent
| | * 9876543 an unrelated commit and child of 8765432
| | * 8765432 ...

> Now the message from you I am responding to in the "Sumary" above
> says that it is not "root" but is about the placement of graph
> nodes.
> 

One and the same issue. Placing an * directly above another * is the issue.

Solution #1

| | * 1234567 commit child of 2345678
| | # 2345678 the first commit, having no parent
| | * 9876543 an unrelated commit and child of 8765432
| | * 8765432 ...

Or

Solution #2

| | * 1234567 commit child of 2345678
| | * 2345678 the first commit, having no parent
| |
| | * 9876543 an unrelated commit and child of 8765432
| | * 8765432 ...

Or

Solution #3

| | * 1234567 commit child of 2345678
| | \
| |  * 2345678 the first commit, having no parent
| | * 9876543 an unrelated commit and child of 8765432
| | * 8765432 ...

All of these solutions will solve the bug. #1 seems to be the easiest and becomes searchable. You have indicated that #3 others have failed to do so. #2 is very much aligned to the --graph without --oneline

> So, I dunno, with changing the description of the goalpost.  Now it
> is that "root" is so not special at all and we only care about that
> the a commit, none of whose parents are in the part of the shown
> history, is shown in such a way that the user can tell that any
> unrelated commits shown in the graph near it are not parents of such
> a commit?  Or do you still want to show such a commit in two ways,
> one for root and one for the ones above the boundary?

A commit without a parent is special - it has no parent. This means it has no history beyond that point. Something special happened at that time - the birth of new source code in source control.

Hopefully, I have cleared up the ambiguous wording.

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

"Jason Pyeron" <jpyeron@pdinc.us> writes:

> One and the same issue. Placing an * directly above another * is the issue.

OK, I re-read the messages in the thread, and it appears that this
part from Kyle

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

was the only thing that argued that A and X (if the graph drawing
happend to place an unrelated commit immediately below it) should be
drawn differently so that you can tell X (root) and A (non root)
apart.

And you are saying (and it seems that you have consistently been
saying) that it is OK to draw A and X (again if other unrelated
commits were immediately drawn below them) the same way.  So I guess
all is well.  We do not have to use more 6 different symbols ("{#}"
to show commit above boundary, three more to show roots) but need to
introduce only three, if we were to go with the Solution #1 route.

It seems to me that Solution #2 is a special case of Solution #3 ;-)
They are both direct answers to the "graph drawn incorrectly can
imply ancestry that does not exist" problem.

Adding the "--decorate-roots" option that annotates the root commits
in the "git log" output can still be done, but that is an orthogonal
issue.  It does solve, together with any one of three options you
presented, the issue Kyle brought up, I would think.

Thanks.

RE: [PATCH 1/2] revision: Denote root commits with '#'

[Jason Pyeron]

> From: Junio C Hamano
> Sent: Saturday, January 23, 2021 6:45 PM
> 
> "Jason Pyeron" writes:
> 
> > One and the same issue. Placing an * directly above another * is the issue.
> 
> OK, I re-read the messages in the thread, and it appears that this
> part from Kyle
> 

Added more of the context below.

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

This was a side track down the left right issue. I personally feel that using the left right features is a buyer beware situation.

> 
> was the only thing that argued that A and X (if the graph drawing
> happend to place an unrelated commit immediately below it) should be
> drawn differently so that you can tell X (root) and A (non root)
> apart.
> 
> And you are saying (and it seems that you have consistently been
> saying) that it is OK to draw A and X (again if other unrelated

I am neither saying or not saying that - partial graph issues are outside of my concerns. Kyle was attempting to reconcile comments on this list about partial graph rendering when his patch was submitted.

> commits were immediately drawn below them) the same way.  So I guess
> all is well.  We do not have to use more 6 different symbols ("{#}"
> to show commit above boundary, three more to show roots) but need to
> introduce only three, if we were to go with the Solution #1 route.

Honestly, I do not care about the <>{}. Whatever makes sense.

> 
> It seems to me that Solution #2 is a special case of Solution #3 ;-)
> They are both direct answers to the "graph drawn incorrectly can
> imply ancestry that does not exist" problem.
> 
> Adding the "--decorate-roots" option that annotates the root commits
> in the "git log" output can still be done, but that is an orthogonal
> issue.  It does solve, together with any one of three options you
> presented, the issue Kyle brought up, I would think.
> 

Yes, adding --decorate-roots to add more wide descriptive text before the message would do it, but it is the worst solution #4.

> Thanks.

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

"Jason Pyeron" <jpyeron@pdinc.us> writes:

>> It seems to me that Solution #2 is a special case of Solution #3 ;-)
>> They are both direct answers to the "graph drawn incorrectly can
>> imply ancestry that does not exist" problem.
>> 
>> Adding the "--decorate-roots" option that annotates the root commits
>> in the "git log" output can still be done, but that is an orthogonal
>> issue.  It does solve, together with any one of three options you
>> presented, the issue Kyle brought up, I would think.
>
> Yes, adding --decorate-roots to add more wide descriptive text
> before the message would do it, but it is the worst solution #4.

I said that "--decorate-roots" is a solution to an orthogonal issue.

Let's recall the C..Z example that shows A (non-root) and X (root)
in several messages back.  Either can be drawn with unrelated commit
immediately below them, depending on the topology of other commits
(imagine there is another commit M that is not related to any of the
commits connected to A or Z, and it is given to "git log C..Z M"; if
we draw C..Z part first and then draw O after it, M would most
likely come immediately after X.

(history: time flows left to right)

          C
         /
        O---A---B
                 \
          X---Y---Z

        M

(log --graph output: time flows bottom to top)

    *   0fbb0dc (HEAD -> z) Z
    |\
    | * 11be529 (master) B
    | * 8dd1b85 A
    * 851a915 Y
    * 27d3ed0 [root] X
    * 1111111 M

Now, the earlier C..Z example I happened to draw B and A first
before drawing Y and X, but if we swap the merge order of Z, it is
likely that the graph output would draw Y and X and then B and A.
"git log C..Z M" in such a history would likely to show M directly
below A (non-root).

    *   0fbb0dc (HEAD -> z) Z
    |\
    | * 851a915 Y
    | * 27d3ed0 [root] X
    * 11be529 (master) B
    * 8dd1b85 A
    * 1111111 M

In short, the [root] annotation does not, and it is not meant to,
solve the "misleading graph" issue.

It only solves "root is special, with or without --graph" issue
(such an issue may or may not exist).

Re: [PATCH 1/2] revision: Denote root commits with '#'

[Junio C Hamano]

Kyle Marek <kmarek@pdinc.us> writes:

> Subject: Re: [PATCH 1/2] revision: Denote root commits with '#'

Downcase "D"; this will stand out in "git shortlog --no-merges" for
a wrong reason otherwise.

> This aids in identifying where an unrelated branch history starts when
> using `git log --graph --oneline --all`

This is triggerd only with --show-linear-break option, when combined
with [2/2]?  I think that is a bug introduced in the next step.

> Signed-off-by: Kyle Marek <kmarek@pdinc.us>
> ---
>  revision.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/revision.c b/revision.c
> index 9dff845bed..8556923de8 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -4191,9 +4191,11 @@ const char *get_revision_mark(const struct rev_info *revs, const struct commit *
>  			return "<";
>  		else
>  			return ">";
> -	} else if (revs->graph)
> +	} else if (revs->graph) {
> +		if (!commit->parents)
> +			return "#";
>  		return "*";
> -	else if (revs->cherry_mark)
> +	} else if (revs->cherry_mark)
>  		return "+";
>  	return "";
>  }

[PATCH 2/2] revision: implement --show-linear-break for --graph

[Kyle Marek]

where <barrier> sets rev_info.break_revision_mark, the revision mark
used for root commits.

Signed-off-by: Kyle Marek <kmarek@pdinc.us>
---
 Documentation/rev-list-options.txt | 7 +++++++
 log-tree.c                         | 2 +-
 revision.c                         | 8 ++++----
 revision.h                         | 1 +
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 002379056a..93adb77c19 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -1104,6 +1104,13 @@ This implies the `--topo-order` option by default, but the
 	do not belong to a linear branch. This option puts a barrier
 	in between them in that case. If `<barrier>` is specified, it
 	is the string that will be shown instead of the default one.
++
+When --graph is used with --oneline, there is usually no vertical
+space between commits, so the graph edge is not drawn. This can make
+it hard to see that a history may end at one commit, while an
+unrelated history starts at the next commit. This option changes the
+revision mark for root commits. If `<barrier>` is specified, it is
+used as the new revision mark instead of the default one.
 
 ifdef::git-rev-list[]
 --count::
diff --git a/log-tree.c b/log-tree.c
index fd0dde97ec..f62300e404 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -962,7 +962,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	if (opt->line_level_traverse)
 		return line_log_print(opt, commit);
 
-	if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)
+	if (!opt->graph && opt->track_linear && !opt->linear && !opt->reverse_output_stage)
 		fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar);
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
diff --git a/revision.c b/revision.c
index 8556923de8..51deab2326 100644
--- a/revision.c
+++ b/revision.c
@@ -2402,10 +2402,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->show_signature = 0;
 	} else if (!strcmp(arg, "--show-linear-break")) {
 		revs->break_bar = "                    ..........";
+		revs->break_revision_mark = "#";
 		revs->track_linear = 1;
 		revs->track_first_time = 1;
 	} else if (skip_prefix(arg, "--show-linear-break=", &optarg)) {
 		revs->break_bar = xstrdup(optarg);
+		revs->break_revision_mark = xstrdup(optarg);
 		revs->track_linear = 1;
 		revs->track_first_time = 1;
 	} else if (skip_prefix(arg, "--show-notes=", &optarg) ||
@@ -2530,8 +2532,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 			unkv[(*unkc)++] = arg;
 		return opts;
 	}
-	if (revs->graph && revs->track_linear)
-		die("--show-linear-break and --graph are incompatible");
 
 	return 1;
 }
@@ -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;
 		return "*";
 	} else if (revs->cherry_mark)
 		return "+";
diff --git a/revision.h b/revision.h
index 086ff10280..83b2ecef56 100644
--- a/revision.h
+++ b/revision.h
@@ -297,6 +297,7 @@ struct rev_info {
 
 	struct commit_list *previous_parents;
 	const char *break_bar;
+	const char *break_revision_mark;
 
 	struct revision_sources *sources;
 
-- 
2.29.2

Re: [PATCH 2/2] revision: implement --show-linear-break for --graph

[Junio C Hamano]

Kyle Marek <kmarek@pdinc.us> writes:

> where <barrier> sets rev_info.break_revision_mark, the revision mark
> used for root commits.

Please make sure that the body of the proposed log message begins
with a full sentence, not as a continuation of a sentence that the
title started (as a consequence, the title must be understandable
without the help of the beginning part of the body, too).

> Signed-off-by: Kyle Marek <kmarek@pdinc.us>
> ---

> diff --git a/revision.c b/revision.c
> index 8556923de8..51deab2326 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2402,10 +2402,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  		revs->show_signature = 0;
>  	} else if (!strcmp(arg, "--show-linear-break")) {
>  		revs->break_bar = "                    ..........";
> +		revs->break_revision_mark = "#";
>  		revs->track_linear = 1;
>  		revs->track_first_time = 1;
>  	} else if (skip_prefix(arg, "--show-linear-break=", &optarg)) {
>  		revs->break_bar = xstrdup(optarg);
> +		revs->break_revision_mark = xstrdup(optarg);
>  		revs->track_linear = 1;
>  		revs->track_first_time = 1;
>  	} else if (skip_prefix(arg, "--show-notes=", &optarg) ||

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.  

The part to teach left-right codepath to show L/R is a fix-up to
[1/2], not to this step.  You might want to change them to some
left/right punctuation letters, like () or [].

The other hunks in revision.c are fixes to step [2/2].

I didn't test a custom --show-linear-break='My break line' in the
attachedtest, so that it can be squashed into your [1/2] to test the
feature that step adds.  You should be able to add tests for that
feature in this step [2/2] on top.

I still am skeptical that spending 3 more letters to denote roots is
worth it, though.

 revision.c                   |  11 ++--
 t/t6020-rev-list-boundary.sh | 132 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 138 insertions(+), 5 deletions(-)

diff --git c/revision.c w/revision.c
index 33fbef5c08..55521c53af 100644
--- c/revision.c
+++ w/revision.c
@@ -2402,7 +2402,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->show_signature = 0;
 	} else if (!strcmp(arg, "--show-linear-break")) {
 		revs->break_bar = "                    ..........";
-		revs->break_revision_mark = "#";
 		revs->track_linear = 1;
 		revs->track_first_time = 1;
 	} else if (skip_prefix(arg, "--show-linear-break=", &optarg)) {
@@ -4219,12 +4218,14 @@ const char *get_revision_mark(const struct rev_info *revs, const struct commit *
 		return "=";
 	else if (!revs || revs->left_right) {
 		if (commit->object.flags & SYMMETRIC_LEFT)
-			return "<";
+			return commit->parents ? "<" : "L";
 		else
-			return ">";
+			return commit->parents ? ">" : "R";
 	} else if (revs->graph) {
-		if (revs->break_revision_mark && !commit->parents)
-			return revs->break_revision_mark;
+		if (!commit->parents)
+			return (revs->break_revision_mark 
+				? revs->break_revision_mark
+				: "#");
 		return "*";
 	} else if (revs->cherry_mark)
 		return "+";
diff --git c/t/t6020-rev-list-boundary.sh w/t/t6020-rev-list-boundary.sh
new file mode 100755
index 0000000000..35614e9baf
--- /dev/null
+++ w/t/t6020-rev-list-boundary.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+
+test_description='rev-list/log boundary and root'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit A &&
+	test_commit B &&
+	git reset --hard A &&
+	test_commit C &&
+
+	git checkout --orphan side &&
+	git rm -fr . &&
+	test_commit X &&
+	test_commit Y &&
+
+	test_tick && git merge --allow-unrelated-histories -m "M" B &&
+	test_tick && git merge -m "N" C &&
+	test_commit Z
+'
+
+test_expect_success 'log with boundary' '
+	git log --graph --boundary --format='%s' ^A ^X Z >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	* Z
+	*   N
+	|\  Q
+	| * C
+	* |   M
+	|\ \  Q
+	| * | B
+	| |/  Q
+	* | Y
+	o | X
+	 /  Q
+	o A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'log --left-right with symmetric boundary' '
+	git log --graph --left-right --boundary --format='%s' B...C >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	> C
+	| < B
+	|/  Q
+	o A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'log --left-right with asymmetric boundary' '
+	git log --graph --left-right --boundary --format='%s' ^A ^X Z >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	> Z
+	>   N
+	|\  Q
+	| > C
+	> |   M
+	|\ \  Q
+	| > | B
+	| |/  Q
+	> | Y
+	o | X
+	 /  Q
+	o A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'log down to root' '
+	git log --graph --format='%s' Z >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	* Z
+	*   N
+	|\  Q
+	| * C
+	* |   M
+	|\ \  Q
+	| * | B
+	| |/  Q
+	| # A
+	* Y
+	# X
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'log down to root' '
+	git log --graph --format='%s' B Y >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	* Y
+	# X
+	* B
+	# A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'log that happens to show root' '
+	git log --graph -3 --format='%s' B Y >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	* Y
+	# X
+	* B
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'log --left-right down to root' '
+	git log --graph --left-right --format='%s' B...Y >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	> Y
+	R X
+	< B
+	L A
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'log --left-right that happens to show root' '
+	git log --graph -3 --left-right --format='%s' B...Y >actual &&
+	sed -e "s/Q$//" >expect <<-\EOF &&
+	> Y
+	R X
+	< B
+	EOF
+	test_cmp expect actual
+'
+
+test_done

Re: [PATCH 2/2] revision: implement --show-linear-break for --graph

[Junio C Hamano]

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.

I still am skeptical that spending 3 more letters to denote roots is
worth it, though.

Thanks.

Re: [PATCH 2/2] revision: implement --show-linear-break for --graph

[Kyle Marek]

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   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Re: [PATCH 2/2] revision: implement --show-linear-break for --graph

[Junio C Hamano]

Kyle Marek <kmarek@pdinc.us> writes:

> Me too, but I think a user-defined mark needs to be a string to
> support Unicode characters.

Ahh, I didn't even consider making it user-defined.

As it seems a lot safer to make this an optional feature, it does
sort-of make sense to let the letters used for root & left-root be
customizable, and it does make sense to take a multi-byte character,
but I am not sure what implications it has if we allowed any string
without ensuring that it occupies one display column.

Re: [PATCH 2/2] revision: implement --show-linear-break for --graph

[Kyle Marek]

On 1/18/21 4:01 PM, Junio C Hamano wrote:
> Kyle Marek <kmarek@pdinc.us> writes:
>
>> Me too, but I think a user-defined mark needs to be a string to
>> support Unicode characters.
> Ahh, I didn't even consider making it user-defined.
>
> As it seems a lot safer to make this an optional feature, it does
> sort-of make sense to let the letters used for root & left-root be
> customizable, and it does make sense to take a multi-byte character,
> but I am not sure what implications it has if we allowed any string
> without ensuring that it occupies one display column.

Does git, or a dependency library, have the ability to interpret TERM 
and locale to determine on-screen character count/size?

If not, maybe let users use multi-character strings, but call it misuse 
of the option that will mess offset that row of the --graph output until 
we have something to determine on-screen size.

-- 

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-                                                               -
- Kyle Marek                        PD Inc. http://www.pdinc.us -
- Jr. Developer                     10 West 24th Street #100    -
- +1 (443) 269-1555 x361            Baltimore, Maryland 21218   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Re: add a blank line when a commit has no parent in log output?

[Junio C Hamano]

"Jason Pyeron" <jpyeron@pdinc.us> writes:

> Is there a way to have it look like:
>
> | | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> | | |
> | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)
>
> Or 
>
> | | | #  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
> | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)

This latter variant won't work.  Imagine we are showing --left-right
for example.  Which side does '#' belong to?

The former is not so great in that it wastes a line, and the break
won't be as noticeable when --graph is *not* used with --oneline.

It would be great to show it more like this:

 | | |   * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
 | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
 | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)

The point being that by shifting the column for the commit to the
right, it shows that 5505 is not a child of 3e65 (and 3e65 is the
tip of its lineage), and its parents do not appear in the displayed
history.  In the real life, the independent 'root' may be connected
to the main history somehow, so you may see a graph like this:

 | | *  12345678 2021-01-14 merge xxxxx@xxxx into the history  
 | | |\
 | | | \
 | | *  \  23456789 2021-01-12 merge citest into the main history
 | | |\  * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
 | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest) Added defau
 | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from f7daf088)


Hmm?