On Tue, 9 Oct 2018 at 21:43, Junio C Hamano <gitster@pobox.com> wrote:

> I had a bit hard time parsing the above, especially with "then",
> which probably would make it easier to read if it is not there.

Okay, I guess better to separate the explanation from the diagrams,
rather than weaving them together:

    For octopus merges where the first parent edge immediately merges into
    the next column to the left, the number of columns should be one less
    than the usual case.

    First parent to the left case:

    | *-.
    | |\ \
    |/ / /

    The usual case:

    | *-.
    | |\ \
    | | | *

> > Also refactor the code to iterate over columns rather than dashes,
> > building from an initial patch suggestion by Jeff King.
>
> s/suggestion/suggested/ perhaps?

Ok.

> It is unclear to me what "delta of columns" means here.  Is this
> because I am unfamiliar with the internal of graph.[ch] API (and
> 'delta of columns' is used elsewhere in the API internals already)?

No, I just meant difference in number of columns from the previous
line to the next. Actually, I had kind of wanted to use "new columns",
but that would be confusing with the num_new_columns variable actually
meaning the total number of columns in the next line.

> > +     int delta_cols = (graph->num_new_columns - graph->num_columns);
>
> So in the second picture above, new-columns (which is the columns
> used after showing the current line) is narrower (because 'x' reuses
> an already allocated column without getting a new one) than columns
> (which is the columns for the octopus merge we are showing)?
>
> I am not sure I follow what is going on around here, sorry.

Maybe it's clearer by saying "added columns" (also expanded the comments a bit)?

    /*
     * Usually, we add one new column for each parent (like the diagram
     * above) but sometimes the first parent goes into an existing column,
     * like this:
     *
     * | *---.
     * | |\ \ \
     * |/ / / /
     * x 0 1 2
     *
     * In which case the number of parents will be one greater than the
     * number of added columns.
     */
    int added_cols = (graph->num_new_columns - graph->num_columns);
    int parent_in_old_cols = graph->num_parents - added_cols;