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

From: Stefan Beller <sbeller@google.com>
Date: Mar 9, 2018, 08:53 UTC
Message-ID: <CAGZ79kZk7N4zQUS1eMFMPTuPPuo5ViOeLj5hQHV=E+A=OO+D0w@mail.gmail.com>
In-Reply-To: <20180307211140.19272-1-rcdailey@gmail.com>

On Wed, Mar 7, 2018 at 1:11 PM, Robert Dailey <rcdailey.lists@gmail.com> wrote:

Show 19 quoted lines

> I am experimenting with a version of submodule diff (using log style)
> that prints the commits brought in from merges, while excluding the
> merge commits themselves. This is useful in cases where a merge commit's
> summary does not fully explain the changes being merged (for example,
> for longer-lived branches).
>
> I could have gone through the effort to make this more configurable, but
> before doing that level of work I wanted to get some discussion going to
> understand first if this is a useful change and second how it should be
> configured. For example, we could allow:
>
> $ git diff --submodule=long-log
>
> Or a supplementary option such as:
>
> $ 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>.

I would think we would want to have different settings per repository, i.e. these settings would only apply to the superproject, however we could keep the same names for submodules, such that we could do

    git log --min-parents=0 --submodules=--no-merges

We started an effort to have a repository object handle in most functions some time ago, but the option parsing for the revision walking doesn't take a repository yet, otherwise the generic revision parsing for submodules would be easy to implement.

Thoughts on this generic approach? Stefan

Show 23 quoted lines

>
> Signed-off-by: Robert Dailey <rcdailey@gmail.com>
> ---
>  submodule.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/submodule.c b/submodule.c
> index 2967704317..a0a62ad7bd 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -428,7 +428,8 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,
>         init_revisions(rev, NULL);
>         setup_revisions(0, NULL, rev, NULL);
>         rev->left_right = 1;
> -       rev->first_parent_only = 1;
> +       rev->max_parents = 1;
> +       rev->first_parent_only = 0;
>         left->object.flags |= SYMMETRIC_LEFT;
>         add_pending_object(rev, &left->object, path);
>         add_pending_object(rev, &right->object, path);
> --
> 2.13.1.windows.2
>