From: Junio C Hamano <gitster@pobox.com>
To: Sergey Organov <sorganov@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] t4013: add expected failure for "log --patch --no-patch"
Date: Thu, 04 May 2023 13:53:30 -0700 [thread overview]
Message-ID: <xmqqpm7fizsl.fsf@gitster.g> (raw)
In-Reply-To: <87o7n03qgq.fsf@osv.gnss.ru> (Sergey Organov's message of "Thu, 04 May 2023 21:24:05 +0300")
Sergey Organov <sorganov@gmail.com> writes:
> [-s meaning "silent" at that time? If so, making --no-patch a synonym for
> "silent", and then documenting -s a synonym for --no-patch sounds like
> quite a twitch.]
I thought it stood for "squelch", but it turns out that it was
introduced by f4f21ce3 (git-diff-tree: clean up output, 2005-05-06),
as a "silent" mode, way before anything else in the thread has
happened.
In Git v1.8.4, "--no-patch" was added as a synonym for "-s", and
ever since we documented that they are equivalents.
> Anyway, this seems pretty irrelevant to the test case. Even
> if we spell --no-patch as -s,
>
> git diff -s --raw HEAD^ HEAD
>
> should produce what? To me it should be the same as
>
> git diff --raw HEAD^ HEAD
>
> as -s turns off everything, and then --raw is turned on.
Ah, yeah, I missed that part of your sample command. It does seem
quite puzzling, and the bad (or good?) part of the story is that my
fresh build of v1.8.4 behaves exactly the same way. And with "-s"
we can try versions before v1.8.4 (the only change that version made
was to introduce "--no-patch" as its synonym). It seems it behaved
that way at least back to v1.5.3 (which happens to be the oldest
version I consider worth comparing with more modern versions).
> Notice that
>
> git diff -s --patch
>
> does produce the patch output, whereas
>
> git diff -s --raw
> git diff -s --stat
>
> produce none.
Yes, you're right.
> Sounds like nonsense.
Sounds like a bug to me. I wonder how it came about. Did we forget
to add support of the equivalent of what "-s --patch" does, when we
added "--raw" and "--stat", perhaps?
> I'm afraid the solution I'd come up with won't be welcomed. If I'd start
> to "fix" it, it'd be likely set of independent options:
>
> --patch --no-patch
> --raw --no-raw
> --stat --no-stat
>
> and then
>
> -s being just a shortcut for "--no-raw --no-patch --no-stat"
If I were writing Git from scratch without any existing users, that
would be how I would design it (modulo that I would make sure we
have some mechanism to make it easier for developers who may add
a new output <format> to ensure that "-s" also implies "--no-<format>"
for the new <format> they are adding to the mix).
The fact that this wasn't brought up until now may mean that nobody
would notice if we redefined the definition of "--no-patch" to
behave that way, as long as "-s" keeps its original meaning.
I dunno.
Thanks.
next prev parent reply other threads:[~2023-05-04 20:54 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-03 13:41 [PATCH] t4013: add expected failure for "log --patch --no-patch" Sergey Organov
2023-05-03 16:57 ` Junio C Hamano
2023-05-03 17:31 ` Sergey Organov
2023-05-03 18:07 ` Junio C Hamano
2023-05-03 18:32 ` Felipe Contreras
2023-05-03 19:49 ` Sergey Organov
2023-05-04 15:50 ` Junio C Hamano
2023-05-04 18:24 ` Sergey Organov
2023-05-04 20:53 ` Junio C Hamano [this message]
2023-05-04 21:37 ` Re* " Junio C Hamano
2023-05-04 23:10 ` [PATCH] diff: fix behaviour of the "-s" option Junio C Hamano
2023-05-05 5:28 ` Junio C Hamano
2023-05-05 16:51 ` Junio C Hamano
2023-05-09 1:16 ` Felipe Contreras
2023-05-05 8:32 ` Sergey Organov
2023-05-05 16:31 ` Junio C Hamano
2023-05-05 17:07 ` Sergey Organov
2023-05-05 16:59 ` [PATCH v2] diff: fix interaction between the "-s" option and other options Junio C Hamano
2023-05-05 17:41 ` Eric Sunshine
2023-05-05 19:01 ` Junio C Hamano
2023-05-05 21:19 ` [PATCH 0/2] dirstat: leakfix Junio C Hamano
2023-05-05 21:19 ` [PATCH 1/2] diff: refactor common tail part of dirstat computation Junio C Hamano
2023-05-05 21:19 ` [PATCH 2/2] diff: plug leaks in dirstat Junio C Hamano
2023-05-09 0:38 ` [PATCH v2] diff: fix interaction between the "-s" option and other options Felipe Contreras
2023-05-09 1:22 ` Junio C Hamano
2023-05-09 3:50 ` Felipe Contreras
2023-05-10 4:26 ` Junio C Hamano
2023-05-10 23:16 ` Felipe Contreras
2023-05-10 23:41 ` Felipe Contreras
2023-05-11 1:25 ` Jeff King
2023-05-13 3:07 ` Felipe Contreras
2023-05-11 1:50 ` Junio C Hamano
2023-05-13 5:32 ` Felipe Contreras
2023-05-09 1:34 ` [PATCH] t4013: add expected failure for "log --patch --no-patch" Felipe Contreras
2023-05-10 13:54 ` Sergey Organov
2023-05-10 21:54 ` Felipe Contreras
2023-05-09 1:03 ` Felipe Contreras
2023-05-04 18:07 ` Junio C Hamano
2023-05-04 18:26 ` Sergey Organov
2023-05-09 1:07 ` Felipe Contreras
2023-05-10 13:40 ` Sergey Organov
2023-05-10 21:39 ` Felipe Contreras
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=xmqqpm7fizsl.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=sorganov@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).