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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 18, 2021, 20:33 UTC
Message-ID: <xmqqr1mij88k.fsf@gitster.c.googlers.com>
In-Reply-To: <xmqqwnwajbuj.fsf@gitster.c.googlers.com>

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

Show 6 quoted lines

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

Show 6 quoted lines

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