All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Taylor Blau <me@ttaylorr.com>,
	Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only
Date: Fri, 5 Feb 2021 05:11:48 -0500	[thread overview]
Message-ID: <YB0Z5JXbe22ko2Mj@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqtuqr2uz3.fsf@gitster.c.googlers.com>

On Thu, Feb 04, 2021 at 04:56:16PM -0800, Junio C Hamano wrote:

> > As an aside: I am curious if I'm missing something when you say the
> > "only way" is to ask for a 'git range-diff ...@{u}'. IIUC what you're
> > describing, I often resort to using 'git cherry' for that exact thing.
> > But, I may not be quite understanding your use-case (and why git-cherry
> > doesn't do what you want already).
> >
> > My latter question is purely for satisfying my own curiosity; I don't
> > have any problem with a '--{left,right}-only' option in range-diff. From
> > my quick read of the patches, it all looks pretty sane to me.
> 
> The question is addressed to Dscho, and I am also somewhat curious.
> Perhaps the reason would be that the output from cherry is not as
> easy to read as range-diff, without any post-processing.

I had the same curiosity; I'd use git-cherry (or rev-list --cherry) for
this.

I suspect the big difference is the quality of the matching. git-cherry
is purely looking at patch-ids. So it is quite likely to say "this was
not applied upstream" when what got applied differed slightly (e.g.,
fixups upstream, applied to a different base, etc). Whereas range-diff
has some cost heuristics for deciding that two patches are basically the
same thing.  So it would find more cases (and as a bonus, give you the
diff to see what tweaks were made upstream).

It does make me wonder if it would be useful for rev-list, etc to have
an option to make "--cherry" use the more clever heuristics instead of
just a patch-id. It would never show the same diff output as range-diff,
but maybe more scripts would find the advanced heuristic useful.

I know it would probably make rebase's "ignore if in upstream" feature
less clunky when I rebase topics. But it would also make it more
dangerous! E.g., right now I see any upstream tweaks as potential
conflicts when I rebase, and I manually review them for sanity.

-Peff

  reply	other threads:[~2021-02-05 10:16 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-04 20:07 [PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only Johannes Schindelin via GitGitGadget
2021-02-04 20:07 ` [PATCH 1/6] range-diff: avoid leaking memory in two error code paths Johannes Schindelin via GitGitGadget
2021-02-04 20:07 ` [PATCH 2/6] range-diff: libify the read_patches() function again Johannes Schindelin via GitGitGadget
2021-02-04 20:07 ` [PATCH 3/6] range-diff: simplify code spawning `git log` Johannes Schindelin via GitGitGadget
2021-02-04 20:07 ` [PATCH 4/6] range-diff: combine all options in a single data structure Johannes Schindelin via GitGitGadget
2021-02-04 23:56   ` Eric Sunshine
2021-02-05 14:13     ` Johannes Schindelin
2021-02-04 20:07 ` [PATCH 5/6] range-diff: move the diffopt initialization down one layer Johannes Schindelin via GitGitGadget
2021-02-04 20:07 ` [PATCH 6/6] range-diff: offer --left-only/--right-only options Johannes Schindelin via GitGitGadget
2021-02-04 22:41 ` [PATCH 0/6] Optionally restrict range-diff output to "left" or "right" range only Junio C Hamano
2021-02-04 22:48   ` Taylor Blau
2021-02-05  0:56     ` Junio C Hamano
2021-02-05 10:11       ` Jeff King [this message]
2021-02-08 22:36         ` Johannes Schindelin
2021-02-05 20:05       ` Taylor Blau
2021-02-05 14:46 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2021-02-05 14:46   ` [PATCH v2 1/6] range-diff: avoid leaking memory in two error code paths Johannes Schindelin via GitGitGadget
2021-02-05 14:46   ` [PATCH v2 2/6] range-diff: libify the read_patches() function again Johannes Schindelin via GitGitGadget
2021-02-05 14:46   ` [PATCH v2 3/6] range-diff: simplify code spawning `git log` Johannes Schindelin via GitGitGadget
2021-02-05 14:46   ` [PATCH v2 4/6] range-diff: combine all options in a single data structure Johannes Schindelin via GitGitGadget
2021-02-05 14:46   ` [PATCH v2 5/6] range-diff: move the diffopt initialization down one layer Johannes Schindelin via GitGitGadget
2021-02-05 14:46   ` [PATCH v2 6/6] range-diff: offer --left-only/--right-only options Johannes Schindelin via GitGitGadget

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

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

  git send-email \
    --in-reply-to=YB0Z5JXbe22ko2Mj@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=me@ttaylorr.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.