git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Git List" <git@vger.kernel.org>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Stefan Beller" <sbeller@google.com>
Subject: Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter
Date: Wed, 25 Jul 2018 15:30:44 -0400	[thread overview]
Message-ID: <CAPig+cS-haNwJha44gncB9PrXRPcEPuqDVGLgKii29h=abq6sA@mail.gmail.com> (raw)
In-Reply-To: <xmqqva93usud.fsf@gitster-ct.c.googlers.com>

On Wed, Jul 25, 2018 at 1:29 PM Junio C Hamano <gitster@pobox.com> wrote:
> A few random thoughts.
>
>  * I find it somewhat disturbing that use of dash is inconsistent
>    between "--interdiff=<arg>" and "--range-diff=<arg>".

I went with the common spellings we've seen on the mailing lists.
"Interdiff", in particular, seems well established. "Range-diff" is
new, so we don't have much data other than what we saw during the
discussion when renaming git-branch-diff, and, of course,
git-branch-diff itself is hyphenated. Another consideration: "inter"
is a prefix, whereas "range" stands on its own.

I don't have a super strong opinion, as I'm used to both the
hyphenated name (from discussion and the command name itself), and the
unhyphenated name ("rangediff" was my local branch name). I'm open to
opinions.

We probably wouldn't want to do so, but another possibility is to
recognize both --range-diff and --rangediff.

>  * Perhaps "--interdiff=<previous> --range-diff" may be a possible
>    way to say "use the same <previous> and show both"?  Do we want
>    "--range-diff=<previous> --interdiff" to mean the same two
>    meta-diff but shown in different order?

That's not at all a bad shorthand. I like it better than the
"--all-kinds-of-diff=<rev>" mentioned earlier. We don't need to make a
decision for this series since such functionality can be added later.

The order of specification of the two options affecting the order of
output is also an interesting idea. We may want to decide that before
graduating this topic.

>  * Do we expect that we may find a third-kind of "meta-diff" that
>    sits next to interdiff and range-diff in the future?  I *think*
>    two separate options "--interdiff=..." and "--range-diff=..." is
>    still a good way forward, and we'd add "--frotzdiff=..." when
>    such a third-kind of "meta-diff" turns out to be useful, without
>    fearing proliferation of options, and that would be OK, but I am
>    just thinking aloud to see if other people have better ideas.

I did consider other approaches, such as a more generic option. For
example, --embed=range-diff:<prev>. I also considered supporting more
complex use-cases. For instance, git-range-diff itself accepts the two
ranges in several formats ("a..b c..d" or "x...z" or "i j k"), plus
the creation-factor can be tweaked, so I weighed ways of incorporating
all that detail into the single argument to --range-diff.

However, those are all difficult to use (onerous to type and remember)
and difficult to document. Thus, I opted for simplicity of individual,
straight-forward options (--interdiff, --range-diff,
--creation-factor), which are easy to type, remember, and document.

We may want to revisit this later if git-format-patch grows additional
similar options.

  reply	other threads:[~2018-07-25 19:30 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-22  9:57 [PATCH 00/14] format-patch: add --interdiff and --range-diff options Eric Sunshine
2018-07-22  9:57 ` [PATCH 01/14] format-patch: allow additional generated content in make_cover_letter() Eric Sunshine
2018-07-22  9:57 ` [PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter Eric Sunshine
2018-07-22 10:31   ` Eric Sunshine
2018-07-23 16:02   ` Duy Nguyen
2018-07-23 19:18     ` Eric Sunshine
2018-07-23 21:36       ` Junio C Hamano
2018-07-22  9:57 ` [PATCH 03/14] format-patch: teach --interdiff to respect -v/--reroll-count Eric Sunshine
2018-07-23 16:12   ` Duy Nguyen
2018-07-23 19:32     ` Eric Sunshine
2018-07-22  9:57 ` [PATCH 04/14] interdiff: teach show_interdiff() to indent interdiff Eric Sunshine
2018-07-22  9:57 ` [PATCH 05/14] log-tree: show_log: make commentary block delimiting reusable Eric Sunshine
2018-07-22  9:57 ` [PATCH 06/14] format-patch: allow --interdiff to apply to a lone-patch Eric Sunshine
2018-07-23 16:22   ` Duy Nguyen
2018-07-23 19:46     ` Eric Sunshine
2018-07-22  9:57 ` [PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout' Eric Sunshine
2018-07-23 22:59   ` Stefan Beller
2018-07-23 23:20     ` Eric Sunshine
2018-07-25 18:25       ` Junio C Hamano
2018-07-22  9:57 ` [PATCH 08/14] range-diff: publish default creation factor Eric Sunshine
2018-07-22  9:57 ` [PATCH 09/14] range-diff: relieve callers of low-level configuration burden Eric Sunshine
2018-07-22  9:57 ` [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter Eric Sunshine
2018-07-23 16:28   ` Duy Nguyen
2018-07-23 19:58     ` Eric Sunshine
2018-07-25 17:29       ` Junio C Hamano
2018-07-25 19:30         ` Eric Sunshine [this message]
2018-07-25 20:32           ` Junio C Hamano
2018-07-25 17:38       ` Duy Nguyen
2018-07-22  9:57 ` [PATCH 11/14] format-patch: extend --range-diff to accept revision range Eric Sunshine
2018-07-25 20:53   ` Junio C Hamano
2018-09-07  9:15     ` Eric Sunshine
2018-07-22  9:57 ` [PATCH 12/14] format-patch: teach --range-diff to respect -v/--reroll-count Eric Sunshine
2018-07-22  9:57 ` [PATCH 13/14] format-patch: add --creation-factor tweak for --range-diff Eric Sunshine
2018-07-22  9:57 ` [PATCH 14/14] format-patch: allow --range-diff to apply to a lone-patch Eric Sunshine
2018-07-25 21:07   ` Junio C Hamano
2018-09-07  8:46     ` Eric Sunshine
2018-07-23 16:32 ` [PATCH 00/14] format-patch: add --interdiff and --range-diff options Duy Nguyen
2018-07-23 20:03   ` Eric Sunshine

Reply instructions:

You may reply publicly 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='CAPig+cS-haNwJha44gncB9PrXRPcEPuqDVGLgKii29h=abq6sA@mail.gmail.com' \
    --to=sunshine@sunshineco.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=sbeller@google.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).