git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format
Date: Tue, 11 Aug 2020 01:22:26 -0400	[thread overview]
Message-ID: <20200811052226.GA82699@coredump.intra.peff.net> (raw)
In-Reply-To: <20200811003359.GD17119@danh.dev>

On Tue, Aug 11, 2020 at 07:33:59AM +0700, Đoàn Trần Công Danh wrote:

> > Yeah, that's what I was getting at: if you care about robust
> > machine-readability, then the full index is the best solution. Reading
> > between the lines, I think the argument may be "using --full-index is
> > too long and therefore ugly, so people like the short-ish names but with
> > a bit of extra safety".
> 
> My argument was people can either easily fetch the patch via HTTP like:
> 
> 	curl -LO https://github.com/git/git/commit/eb12adc74cf22add318f884072be2071d181abaa.patch
> 
> or take it from a mailing list archive, bugzilla, instead of
> cloning a full repository. With those options, we can't say,
> "we prefer full-index, please send us the patch with full-index
> instead".

OK. But then how would they use "--abbrev" in that case? I.e., isn't it
too late at that point (especially in the mailing list archive case) to
do change anything in the formatting of the patch?

Maybe I'm confused...

> > There's an extra challenge here, which is that you have to convince the
> > sender to use the extra --abbrev option, even though they themselves
> > won't be the ones running into the problem when applying.
> 
> Not really, since the sender tree is usually larger than the archived
> tree, their abbrev is usually long enough, and the receiver will use
> --abbrev to lengthen their abbrev to reduce the noise instead.

Now I'm doubly confused. If the sender has the larger tree then they'll
have the larger abbrev. So what's the problem?

Going back to re-read your earlier responses...So...this _isn't_ a
problem within Git itself? It's only about people trying to compare
textual patches byte-for-byte and seeing different index lines?

If that's the case, then it seems to me that the byte comparison is the
problem here. If I have:

  index 1234abcd..5678bcde

and

  index 1234abcd87..5678bcde65

those should be considered equivalent to see if two patches are
plausibly the same. And I think tools like git-cherry, etc, would do
that (and we provide git-patch-id for that purpose, too).

> > Yeah, I certainly don't mind the extra flexibility between "full" and
> > "default" for "index" lines. I do wonder if people want to configure the
> > abbreviations for those lines separately from other parts. I don't know
> > that I've ever particularly cared about that flexibility, but the fact
> > that they were set up separately all those years ago makes me think
> > somebody might.
> 
> I don't think people particularly care about the index line (and to
> the extent, its length) that much, since the default is number is
> actually a minimum number, if Git can't differentiate object with that
> number of characters, Git will show a longer object names anyway.
> 
> I think most people scripts will put a regex for:
> 
> 	/index [a-z0-9]{7,}\.\.[a-z0-9]{7,} [0-7]{6}/
> 
> Or even:
> 
> 	/index [a-z0-9]+\.\.[a-z0-9]+ [0-7]+/
> 
> For the former case, we could change the code in 2/2 to set the minimum
> default to DEFAULT_ABBREV instead of MINIMUM_ABBREV?
> 
> For the historical case that users put both --full-index and --abbrev
> into there scripts, we still keep our promise to not break their
> script by always respect --full-index, regardless of --abbrev.

I care less about scripting (as you note, anything consuming abbreviated
objects has to handle longer-than-minimum names anyway), and was more
wondering whether anybody really cared that:

 git log --abbrev=30 -p

kept the short index lines (e.g., because they're easier to read). But
I'm having trouble coming up with a plausible reason somebody would want
long object names in earlier lines like "Merge:" but not in the patch
index lines. And already we respect --abbrev for --raw, so it's not like
the diff code isn't already affected. Making "-p" consistent with all
the rest of it is probably worth doing regardless.

-Peff

  reply	other threads:[~2020-08-11  5:22 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-09  2:19 [RFC PATCH 0/2] extend --abbrev support to diff-patch format Đoàn Trần Công Danh
2020-08-09  2:19 ` [RFC PATCH 1/2] revision: differentiate if --no-abbrev asked explicitly Đoàn Trần Công Danh
2020-08-09  2:19 ` [RFC PATCH 2/2] diff: extend --abbrev support to diff-patch format Đoàn Trần Công Danh
2020-08-09 19:01 ` [RFC PATCH 0/2] " Junio C Hamano
2020-08-10 10:00   ` Jeff King
2020-08-10 12:31     ` Đoàn Trần Công Danh
2020-08-10 15:15       ` Junio C Hamano
2020-08-10 15:27         ` Jeff King
2020-08-11  0:33           ` Đoàn Trần Công Danh
2020-08-11  5:22             ` Jeff King [this message]
2020-08-11 12:07               ` Đoàn Trần Công Danh
2020-08-10 15:06     ` Junio C Hamano
2020-08-11 11:49 ` [PATCH v2 0/2] diff: index-line: respect --abbrev in object's name Đoàn Trần Công Danh
2020-08-11 11:49   ` [PATCH v2 1/2] revision: differentiate if --no-abbrev asked explicitly Đoàn Trần Công Danh
2020-08-11 18:54     ` Junio C Hamano
2020-08-11 11:49   ` [PATCH v2 2/2] diff: index-line: respect --abbrev in object's name Đoàn Trần Công Danh
2020-08-14  0:23 ` [PATCH v3 0/2] " Đoàn Trần Công Danh
2020-08-14  0:23   ` [PATCH v3 1/2] revision: differentiate if --no-abbrev asked explicitly Đoàn Trần Công Danh
2020-08-14  0:50     ` Junio C Hamano
2020-08-14  0:59       ` Đoàn Trần Công Danh
2020-08-14  1:06         ` Junio C Hamano
2020-08-14 14:50           ` Đoàn Trần Công Danh
2020-08-19 22:50             ` Junio C Hamano
2020-08-14  0:23   ` [PATCH v3 2/2] diff: index-line: respect --abbrev in object's name Đoàn Trần Công Danh
2020-08-14 15:18     ` SZEDER Gábor
2020-08-14 17:00       ` Junio C Hamano
2020-08-14 18:59         ` Junio C Hamano
2020-08-15  0:21           ` brian m. carlson
2020-08-15  2:27             ` Đoàn Trần Công Danh
2020-08-17 16:17               ` Junio C Hamano
2020-08-20  4:56               ` Junio C Hamano
2020-08-20 12:35 ` [PATCH v4 0/2] " Đoàn Trần Công Danh
2020-08-20 12:35   ` [PATCH v4 1/2] t4013: improve diff-post-processor logic Đoàn Trần Công Danh
2020-08-20 19:49     ` Junio C Hamano
2020-08-21 12:05       ` Đoàn Trần Công Danh
2020-08-21 15:44         ` Junio C Hamano
2020-08-20 12:35   ` [PATCH v4 2/2] diff: index-line: respect --abbrev in object's name Đoàn Trần Công Danh
2020-08-20 19:58     ` Junio C Hamano
2020-08-21 11:51 ` [PATCH v5 0/2] " Đoàn Trần Công Danh
2020-08-21 11:51   ` [PATCH v5 1/2] t4013: improve diff-post-processor logic Đoàn Trần Công Danh
2020-08-21 11:51   ` [PATCH v5 2/2] diff: index-line: respect --abbrev in object's name Đoàn Trần Công Danh

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=20200811052226.GA82699@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).