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

2 messages, 2 authors, 2021-01-18 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2021-01-17 21:11:12

Kyle Marek [off-list ref] writes:
This aids in identifying where an unrelated branch history starts when
using `git log --graph --oneline --all`

Signed-off-by: Kyle Marek <redacted>
---
 revision.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
No tests?
quoted hunk
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 '#'

From: Kyle Marek <hidden>
Date: 2021-01-18 07:57:10

On 1/17/21 4:10 PM, Junio C Hamano wrote:
Kyle Marek[off-list ref]  writes:
quoted
This aids in identifying where an unrelated branch history starts when
using `git log --graph --oneline --all`

Signed-off-by: Kyle Marek<redacted>
---
  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.
quoted
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   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help