Re: [PATCH] Support long format for log-based submodule diff

From: Stefan Beller <sbeller@google.com>
Date: Mar 27, 2018, 22:17 UTC
Message-ID: <CAGZ79kZ_j3_mhk5asNEBgBe_2qD7=18foJgW=p0+p=uJa3U2nw@mail.gmail.com>
In-Reply-To: <xmqqina56t8h.fsf@gitster-ct.c.googlers.com>

Show 7 quoted lines

> >> $ git diff --submodule=log --submodule-log-detail=(long|short)
> >>
> >> I'm not sure what makes sense here. I welcome thoughts/discussion and
> >> will provide follow-up patches.
> >
> > The case of merges is usually configured with --[no-]merges, or
> > --min-parents=<n>.

> But that is a knob that controls an irrelevant aspect of the detail
> in the context of this discussion, isn't it?  This code is about "to
> what degree the things that happened between two submodule commits
> in an adjacent pair of commits in the superproject are summarized?"

And I took it a step further and wanted to give a general solution, which allows giving any option that the diff machinery accepts to only apply to the submodule diffing part of the current diff.

Show 11 quoted lines

> The hack Robert illustrates below is to change it to stop favouring
> such projects with "clean" histories, and show "log --oneline
> --no-merges --left-right".  When presented that way, clean histories
> of topic-branch based projects will suffer by losing conciseness,
> but clean histories of totally linear projects will still be shown
> the same way, and messy history that sometimes merges, sometimes
> merges mergy histories, and sometimes directly builds on the trunk
> will be shown as an enumeration of individual commits in a flat way
> by ignoring merges and not restricting the traversal to the first
> parent chains, which would appear more uniform than what the current
> code shows.

Oh, I realize this is in the *summary* code path, I was thinking about the show_submodule_inline_diff, which would benefit from more diff options.

> I do not see a point in introducing --min/max-parents as a knob to
> control how the history is summarized.

For a summary a flat list of commits may be fine, ignoring (ideally non-evil) merges.

Show 5 quoted lines

> This is a strongly related tangent, but I wonder if we can and/or
> want to share more code with the codepath that prepares the log
> message for a merge.  It summarizes what happened on the side branch
> since it forked from the history it is joining back to (I think it
> is merge.c::shortlog() that computes this)

I do not find code there. To me it looks like builtin/fmt-merge-msg.c is responsible for coming up with a default merge message? In that file there is a shortlog() function, which walks revisions and puts together the subject lines of commits.

Show 9 quoted lines

> and it is quite similar
> to what Robert wants to use for submodules here.  On the other hand,
> in a project _without_ submodule, if you are pulling history made by
> your lieutenant whose history is full of linear merges of topic
> branches to the mainline, it may not be a bad idea to allow
> fmt-merge-msg to alternatively show something similar to the "diff
> --submodule=log" gives us, i.e. summarize the history of the side
> branch being merged by just listing the commits on the first-parent
> chain.  So I sense some opportunity for cross pollination here.

The cross pollination that I sense is the desire in both cases to freely specify the format as it may depend on the workflow.

Stefan