From: Kyle Marek Date: Tue, 19 Jan 2021 07:43:57 GMT Subject: Re: [PATCH 1/2] revision: Denote root commits with '#' Message-ID: <237aeef3-239f-bff4-fa17-5581092c8f51@pdinc.us> In-Reply-To: On 1/18/21 3:33 PM, Junio C Hamano wrote: > Junio C Hamano 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=" to optionally mark root commits with a string , 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=" and "--mark-right-root=", 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 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-