Re: [PATCH] fixup! graph: output padding for merge subsequent parents
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 10, 2013, 22:38 UTC
- Message-ID
- <7vwqufrdzd.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20130210210229.GB2270@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
Show 20 quoted lines
> On Sun, Feb 10, 2013 at 11:30:39AM -0800, Junio C Hamano wrote: > ... >> Is it correct to say that this essentially re-does 656197ad3805 >> (graph.c: infinite loop in git whatchanged --graph -m, 2009-07-25) >> in a slightly different way, in that Michał's original fix also >> protected against the case where graph->state is flipped to >> GRAPH_PADDING by graph_next_line() that returns false, but with your >> fixup, the code knows it never happens (i.e. when graph_next_line() >> returns false, graph->state is always in the GRAPH_PADDING state), >> and the only thing we need to be careful about is when graph->state >> is already in the PADDING state upon entry to this function? > > Yes, although I wonder if we can end up in POST_MERGE or COLLAPSING > state here as well. The check in the loop guards against that because > those will eventually end up as PADDING. > > As far as I can see, this is okay because we have called > graph_show_remainder() at the end of outputting a commit, even when we > end up outputting the same (merge) commit more than once. But someone > more familiar with the graph code might want to comment here.
More importantly, that kind of thought process needs to be documented in the log message; that will help people to diagnose the cause of the problem if they later find that this patch made an incorrect assumption while simplifying the code.