git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Robert Dailey <rcdailey.lists@gmail.com>,
	git <git@vger.kernel.org>, Stefan Beller <stefanbeller@gmail.com>,
	Brandon Williams <bmwill@google.com>
Subject: Re: [PATCH] Support long format for log-based submodule diff
Date: Tue, 27 Mar 2018 22:17:57 +0000
Message-ID: <CAGZ79kZ_j3_mhk5asNEBgBe_2qD7=18foJgW=p0+p=uJa3U2nw@mail.gmail.com> (raw)
In-Reply-To: <xmqqina56t8h.fsf@gitster-ct.c.googlers.com>

> >> $ 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.

> 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.

> 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.

> 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

  reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-07 21:11 Robert Dailey
2018-03-07 21:41 ` Junio C Hamano
2018-03-09  8:53 ` Stefan Beller
2018-03-09 17:42   ` Junio C Hamano
2018-03-27 22:17     ` Stefan Beller [this message]
2018-04-02  1:07       ` Robert Dailey
2018-04-02 19:35         ` Stefan Beller

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGZ79kZ_j3_mhk5asNEBgBe_2qD7=18foJgW=p0+p=uJa3U2nw@mail.gmail.com' \
    --to=sbeller@google.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=rcdailey.lists@gmail.com \
    --cc=stefanbeller@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox