From: Junio C Hamano <gitster@pobox.com>
To: Sergey Organov <sorganov@gmail.com>
Cc: Felipe Contreras <felipe.contreras@gmail.com>,
git@vger.kernel.org, Matthieu Moy <matthieu.moy@univ-lyon1.fr>
Subject: Re: Can we clarify the purpose of `git diff -s`?
Date: Thu, 11 May 2023 10:37:22 -0700 [thread overview]
Message-ID: <xmqqwn1ewyzx.fsf@gitster.g> (raw)
In-Reply-To: <871qjn2i63.fsf@osv.gnss.ru> (Sergey Organov's message of "Thu, 11 May 2023 14:59:00 +0300")
Sergey Organov <sorganov@gmail.com> writes:
> I entirely agree with your conclusion: obviously, -s (--silent) and
> --no-patch are to be different for UI to be even remotely intuitive, and
> I'd vote for immediate fix of --no-patch semantics even though it's a
> backward-incompatible change.
I forgot to write about this part.
tl;dr. While I do not think the current "--no-patch" that turns off
things other than "--patch" is intuitive, an "immediate" change is
not possible. Let's do one fix at a time.
The behaviour came in the v1.8.4 days with a series that was merged
by e2ecd252 (Merge branch 'mm/diff-no-patch-synonym-to-s',
2013-07-22), which
* made "--no-patch" a synonym for "-s";
* fixed "-s --patch", in which the effect of "-s" got stuck and did
not allow the patch output to be re-enabled again with "--patch";
* updated documentation to explain "--no-patch" as a synonym for
"-s".
While it is very clear that the intent of the author was to make it
a synonym for "-s" and not a "feature-wise enable/disable" option,
that is what we've run with for the past 10 years. You identified
bugs related the "-s got stuck" problem and we recently fixed that.
"Should --no-patch be changed" can be treated as a separate issue,
and whenever we can treat two things separately, I want to do so, to
keep the potential blast radius smaller. That way, if an earlier
change turns out OK but the other change causes severe regression,
we can only revert or rework the latter. An exception is if
committing to one change (e.g. "fix '-s'") makes the other change
(e.g. "redefine --no-patch") impossible, but we all know it is not
the case here. I gave an outline of how to go about it in the log
message of that "fix '-s'" patch.
I do not think it will break established use cases too badly to fix
the behaviour of "-s" so that it does not get stuck. We saw an
existing breakage in one test, but asking the owners of scripts that
make the same mistake of assuming "-s" gets stuck for some but not
other options to fix that assumption based on an earlier faulty
implementation is much easier.
But "git diff --stat --patch --no-patch", which suddenly starts
showing diffstat after you make "--no-patch" no longer a synonym for
"-s", has a much larger potential to break the existing workflows.
And I do not think asking the users who followed the documented
"--no-patch is a synonym for -s" to change their script because we
decided to make "--no-patch" no longer a synonym is much harder.
So, no, I do not think we can immediately "fix". I do not think
anybody knows if it can be done "immediately" or needs a careful
planning to transition, and I offhand do not know if it is even
possible to transition without fallout.
THanks.
next prev parent reply other threads:[~2023-05-11 17:37 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-11 3:14 Can we clarify the purpose of `git diff -s`? Felipe Contreras
2023-05-11 11:59 ` Sergey Organov
2023-05-11 16:26 ` Junio C Hamano
2023-05-11 17:37 ` Junio C Hamano [this message]
2023-05-11 18:04 ` Sergey Organov
2023-05-11 18:27 ` Junio C Hamano
2023-05-11 18:36 ` Felipe Contreras
2023-05-11 18:17 ` Felipe Contreras
2023-05-11 17:41 ` Felipe Contreras
2023-05-11 18:31 ` Sergey Organov
2023-05-11 19:10 ` Felipe Contreras
2023-05-11 19:32 ` Sergey Organov
2023-05-11 19:54 ` Felipe Contreras
2023-05-11 20:24 ` Sergey Organov
2023-05-11 20:59 ` Felipe Contreras
2023-05-11 22:49 ` Sergey Organov
2023-05-11 23:28 ` Felipe Contreras
2023-05-12 8:40 ` Sergey Organov
2023-05-12 19:19 ` Felipe Contreras
[not found] ` <5bb24e0208dd4a8ca5f6697d578f3ae0@SAMBXP02.univ-lyon1.fr>
2023-05-12 8:15 ` Matthieu Moy
2023-05-12 17:03 ` Junio C Hamano
2023-05-12 18:21 ` Sergey Organov
2023-05-12 19:21 ` Junio C Hamano
2023-05-12 19:34 ` Junio C Hamano
2023-05-12 20:28 ` Felipe Contreras
2023-05-12 20:47 ` Junio C Hamano
2023-05-12 21:01 ` Felipe Contreras
2023-05-12 21:47 ` Junio C Hamano
2023-05-12 21:48 ` Junio C Hamano
2023-05-12 23:21 ` Felipe Contreras
2023-05-12 21:41 ` Sergey Organov
2023-05-12 22:17 ` Junio C Hamano
2023-05-12 22:47 ` Sergey Organov
2023-05-12 23:07 ` Felipe Contreras
2023-05-13 14:58 ` Philip Oakley
2023-05-13 17:45 ` Sergey Organov
2023-05-12 19:47 ` Felipe Contreras
2023-05-12 19:34 ` Felipe Contreras
2023-05-12 19:17 ` 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=xmqqwn1ewyzx.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=matthieu.moy@univ-lyon1.fr \
--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).