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

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

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

From: Junio C Hamano <hidden>
Date: 2021-01-18 20:34:46

Junio C Hamano [off-list ref] writes:
[Footnote]

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

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

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

Continuing in the same repository:

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

These depict two commits, A and Xroot, and no other commits.  We
also want to show that these two commits do not have parent-child
relationship at all, but if we paint Xroot with "#", it still makes
it appear that A is a child of Xroot.
    And the right way to look at it is "does A have any parent in
    the part of the history being shown?", not "does A have any
    parent?"  Then 'A' will get exactly the same treatment in the
    two examples, and the visual problem that makes A appear as if
    it has parent-child relationship with unrelated commit X goes
    away.
So the condition we saw in your patches, !commit->parents, which
attempted to see if it was root, needs to be replaced with a helper
function that checks if there is any parent that is shown in the
output.  Perhaps

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

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

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

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

would become

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

Hmm?

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

From: Kyle Marek <hidden>
Date: 2021-01-19 07:56:07

On 1/18/21 3:33 PM, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
[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.
quoted
     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   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help