* add a blank line when a commit has no parent in log output? @ 2021-01-14 18:30 Jason Pyeron 2021-01-14 19:29 ` Philippe Blain 2021-01-15 1:12 ` add a blank line when a commit has no parent in log output? Junio C Hamano 0 siblings, 2 replies; 29+ messages in thread From: Jason Pyeron @ 2021-01-14 18:30 UTC (permalink / raw) To: git 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: add a blank line when a commit has no parent in log output? 2021-01-14 18:30 add a blank line when a commit has no parent in log output? Jason Pyeron @ 2021-01-14 19:29 ` Philippe Blain 2021-01-14 20:44 ` Jason Pyeron 2021-01-15 1:12 ` add a blank line when a commit has no parent in log output? Junio C Hamano 1 sibling, 1 reply; 29+ messages in thread From: Philippe Blain @ 2021-01-14 19:29 UTC (permalink / raw) To: git, Jason Pyeron 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: add a blank line when a commit has no parent in log output? 2021-01-14 19:29 ` Philippe Blain @ 2021-01-14 20:44 ` Jason Pyeron 2021-01-17 11:03 ` [PATCH 0/2] Option to modify revision mark for root commits Kyle Marek 0 siblings, 1 reply; 29+ messages in thread From: Jason Pyeron @ 2021-01-14 20:44 UTC (permalink / raw) To: Kyle Marek; +Cc: 'Philippe Blain', git 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/2] Option to modify revision mark for root commits 2021-01-14 20:44 ` Jason Pyeron @ 2021-01-17 11:03 ` Kyle Marek 2021-01-17 11:03 ` [PATCH 1/2] revision: Denote root commits with '#' Kyle Marek 2021-01-17 11:03 ` [PATCH 2/2] revision: implement --show-linear-break for --graph Kyle Marek 0 siblings, 2 replies; 29+ messages in thread From: Kyle Marek @ 2021-01-17 11:03 UTC (permalink / raw) To: Jason Pyeron, git; +Cc: Philippe Blain 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/2] revision: Denote root commits with '#' 2021-01-17 11:03 ` [PATCH 0/2] Option to modify revision mark for root commits Kyle Marek @ 2021-01-17 11:03 ` Kyle Marek 2021-01-17 21:10 ` Junio C Hamano 2021-01-17 22:44 ` Junio C Hamano 2021-01-17 11:03 ` [PATCH 2/2] revision: implement --show-linear-break for --graph Kyle Marek 1 sibling, 2 replies; 29+ messages in thread From: Kyle Marek @ 2021-01-17 11:03 UTC (permalink / raw) To: Jason Pyeron, git; +Cc: Philippe Blain 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-17 11:03 ` [PATCH 1/2] revision: Denote root commits with '#' Kyle Marek @ 2021-01-17 21:10 ` Junio C Hamano 2021-01-18 7:56 ` Kyle Marek 2021-01-17 22:44 ` Junio C Hamano 1 sibling, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-17 21:10 UTC (permalink / raw) To: Kyle Marek; +Cc: Jason Pyeron, git, Philippe Blain 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-17 21:10 ` Junio C Hamano @ 2021-01-18 7:56 ` Kyle Marek 2021-01-18 19:15 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Kyle Marek @ 2021-01-18 7:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jason Pyeron, git, Philippe Blain 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 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-18 7:56 ` Kyle Marek @ 2021-01-18 19:15 ` Junio C Hamano 2021-01-18 20:33 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-18 19:15 UTC (permalink / raw) To: Kyle Marek; +Cc: Jason Pyeron, git, Philippe Blain 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-18 19:15 ` Junio C Hamano @ 2021-01-18 20:33 ` Junio C Hamano 2021-01-19 7:43 ` Kyle Marek 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-18 20:33 UTC (permalink / raw) To: Kyle Marek; +Cc: Jason Pyeron, git, Philippe Blain 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? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-18 20:33 ` Junio C Hamano @ 2021-01-19 7:43 ` Kyle Marek 2021-01-19 22:10 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Kyle Marek @ 2021-01-19 7:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jason Pyeron, git, Philippe Blain 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 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-19 7:43 ` Kyle Marek @ 2021-01-19 22:10 ` Junio C Hamano 2021-01-20 3:25 ` Kyle Marek 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-19 22:10 UTC (permalink / raw) To: Kyle Marek; +Cc: Jason Pyeron, git, Philippe Blain 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-19 22:10 ` Junio C Hamano @ 2021-01-20 3:25 ` Kyle Marek 2021-01-20 6:47 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Kyle Marek @ 2021-01-20 3:25 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jason Pyeron, git, Philippe Blain 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 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-20 3:25 ` Kyle Marek @ 2021-01-20 6:47 ` Junio C Hamano 2021-01-20 15:11 ` Jason Pyeron 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-20 6:47 UTC (permalink / raw) To: Kyle Marek; +Cc: Jason Pyeron, git, Philippe Blain 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-20 6:47 ` Junio C Hamano @ 2021-01-20 15:11 ` Jason Pyeron 2021-01-20 21:52 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Jason Pyeron @ 2021-01-20 15:11 UTC (permalink / raw) To: git Cc: 'Philippe Blain', 'Junio C Hamano', 'Kyle Marek' > -----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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-20 15:11 ` Jason Pyeron @ 2021-01-20 21:52 ` Junio C Hamano 2021-01-20 23:01 ` Jason Pyeron 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-20 21:52 UTC (permalink / raw) To: Jason Pyeron; +Cc: git, 'Philippe Blain', 'Kyle Marek' "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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-20 21:52 ` Junio C Hamano @ 2021-01-20 23:01 ` Jason Pyeron 2021-01-23 18:07 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Jason Pyeron @ 2021-01-20 23:01 UTC (permalink / raw) To: 'Junio C Hamano' Cc: git, 'Philippe Blain', 'Kyle Marek' 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-20 23:01 ` Jason Pyeron @ 2021-01-23 18:07 ` Junio C Hamano 2021-01-23 23:02 ` Jason Pyeron 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-23 18:07 UTC (permalink / raw) To: Jason Pyeron; +Cc: git, 'Philippe Blain', 'Kyle Marek' "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? ^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-23 18:07 ` Junio C Hamano @ 2021-01-23 23:02 ` Jason Pyeron 2021-01-23 23:45 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Jason Pyeron @ 2021-01-23 23:02 UTC (permalink / raw) To: git Cc: 'Philippe Blain', 'Kyle Marek', 'Junio C Hamano' > -----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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-23 23:02 ` Jason Pyeron @ 2021-01-23 23:45 ` Junio C Hamano 2021-01-24 0:02 ` Jason Pyeron 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-23 23:45 UTC (permalink / raw) To: Jason Pyeron; +Cc: git, 'Philippe Blain', 'Kyle Marek' "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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-23 23:45 ` Junio C Hamano @ 2021-01-24 0:02 ` Jason Pyeron 2021-01-25 7:00 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Jason Pyeron @ 2021-01-24 0:02 UTC (permalink / raw) To: git Cc: 'Philippe Blain', 'Kyle Marek', 'Junio C Hamano' > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-24 0:02 ` Jason Pyeron @ 2021-01-25 7:00 ` Junio C Hamano 0 siblings, 0 replies; 29+ messages in thread From: Junio C Hamano @ 2021-01-25 7:00 UTC (permalink / raw) To: Jason Pyeron; +Cc: git, 'Philippe Blain', 'Kyle Marek' "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). ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] revision: Denote root commits with '#' 2021-01-17 11:03 ` [PATCH 1/2] revision: Denote root commits with '#' Kyle Marek 2021-01-17 21:10 ` Junio C Hamano @ 2021-01-17 22:44 ` Junio C Hamano 1 sibling, 0 replies; 29+ messages in thread From: Junio C Hamano @ 2021-01-17 22:44 UTC (permalink / raw) To: Kyle Marek; +Cc: Jason Pyeron, git, Philippe Blain 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 ""; > } ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/2] revision: implement --show-linear-break for --graph 2021-01-17 11:03 ` [PATCH 0/2] Option to modify revision mark for root commits Kyle Marek 2021-01-17 11:03 ` [PATCH 1/2] revision: Denote root commits with '#' Kyle Marek @ 2021-01-17 11:03 ` Kyle Marek 2021-01-17 22:56 ` Junio C Hamano 1 sibling, 1 reply; 29+ messages in thread From: Kyle Marek @ 2021-01-17 11:03 UTC (permalink / raw) To: Jason Pyeron, git; +Cc: Philippe Blain 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] revision: implement --show-linear-break for --graph 2021-01-17 11:03 ` [PATCH 2/2] revision: implement --show-linear-break for --graph Kyle Marek @ 2021-01-17 22:56 ` Junio C Hamano 2021-01-18 2:09 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-17 22:56 UTC (permalink / raw) To: Kyle Marek; +Cc: Jason Pyeron, git, Philippe Blain 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] revision: implement --show-linear-break for --graph 2021-01-17 22:56 ` Junio C Hamano @ 2021-01-18 2:09 ` Junio C Hamano 2021-01-18 7:56 ` Kyle Marek 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-18 2:09 UTC (permalink / raw) To: Kyle Marek; +Cc: Jason Pyeron, git, Philippe Blain 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] revision: implement --show-linear-break for --graph 2021-01-18 2:09 ` Junio C Hamano @ 2021-01-18 7:56 ` Kyle Marek 2021-01-18 21:01 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Kyle Marek @ 2021-01-18 7:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jason Pyeron, git, Philippe Blain 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 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] revision: implement --show-linear-break for --graph 2021-01-18 7:56 ` Kyle Marek @ 2021-01-18 21:01 ` Junio C Hamano 2021-01-19 7:44 ` Kyle Marek 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2021-01-18 21:01 UTC (permalink / raw) To: Kyle Marek; +Cc: Jason Pyeron, git, Philippe Blain 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] revision: implement --show-linear-break for --graph 2021-01-18 21:01 ` Junio C Hamano @ 2021-01-19 7:44 ` Kyle Marek 0 siblings, 0 replies; 29+ messages in thread From: Kyle Marek @ 2021-01-19 7:44 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jason Pyeron, git, Philippe Blain 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 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: add a blank line when a commit has no parent in log output? 2021-01-14 18:30 add a blank line when a commit has no parent in log output? Jason Pyeron 2021-01-14 19:29 ` Philippe Blain @ 2021-01-15 1:12 ` Junio C Hamano 1 sibling, 0 replies; 29+ messages in thread From: Junio C Hamano @ 2021-01-15 1:12 UTC (permalink / raw) To: Jason Pyeron; +Cc: git "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? ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2021-01-25 7:09 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-14 18:30 add a blank line when a commit has no parent in log output? Jason Pyeron 2021-01-14 19:29 ` Philippe Blain 2021-01-14 20:44 ` Jason Pyeron 2021-01-17 11:03 ` [PATCH 0/2] Option to modify revision mark for root commits Kyle Marek 2021-01-17 11:03 ` [PATCH 1/2] revision: Denote root commits with '#' Kyle Marek 2021-01-17 21:10 ` Junio C Hamano 2021-01-18 7:56 ` Kyle Marek 2021-01-18 19:15 ` Junio C Hamano 2021-01-18 20:33 ` Junio C Hamano 2021-01-19 7:43 ` Kyle Marek 2021-01-19 22:10 ` Junio C Hamano 2021-01-20 3:25 ` Kyle Marek 2021-01-20 6:47 ` Junio C Hamano 2021-01-20 15:11 ` Jason Pyeron 2021-01-20 21:52 ` Junio C Hamano 2021-01-20 23:01 ` Jason Pyeron 2021-01-23 18:07 ` Junio C Hamano 2021-01-23 23:02 ` Jason Pyeron 2021-01-23 23:45 ` Junio C Hamano 2021-01-24 0:02 ` Jason Pyeron 2021-01-25 7:00 ` Junio C Hamano 2021-01-17 22:44 ` Junio C Hamano 2021-01-17 11:03 ` [PATCH 2/2] revision: implement --show-linear-break for --graph Kyle Marek 2021-01-17 22:56 ` Junio C Hamano 2021-01-18 2:09 ` Junio C Hamano 2021-01-18 7:56 ` Kyle Marek 2021-01-18 21:01 ` Junio C Hamano 2021-01-19 7:44 ` Kyle Marek 2021-01-15 1:12 ` add a blank line when a commit has no parent in log output? Junio C Hamano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.