From: Felipe Contreras <felipe.contreras@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2] diff: fix interaction between the "-s" option and other options
Date: Mon, 08 May 2023 21:50:40 -0600 [thread overview]
Message-ID: <6459c31038e81_7c68294ee@chronos.notmuch> (raw)
In-Reply-To: <xmqqsfc62t8y.fsf@gitster.g>
Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> >> Let's fix the interactions of these bits to first make "-s" work as
> >> intended.
> >
> > Is it though?
>
> Yes.
>
> If the proposed log message says "as intended", the author thinks it
> is.
The question is not if the author of the patch thinks this is the way
`-s` is intended to work, the question is if this is the way `-s` is
intended to work.
The way `-s` is intended to work is completely independent of what the
author of the patch thinks, as `-s` existed well before this patch.
A cursory search for `-s` in diff-tree.c shows:
Author: Linus Torvalds <torvalds@ppc970.osdl.org>
Date: Fri May 6 10:56:35 2005 -0700
git-diff-tree: clean up output
This only shows the tree headers when something actually changed. Also,
add a "silent" mode, which doesn't actually show the changes at all,
just the commit information.
So presumably the original author of `-s` intended for it to not show
any changes at all, but that was before any of the non-patch options
were introduced.
So, 18 years later: what is the intention behind `-s`?
> Throwing a rhetorical question and stopping at that is not
> useful;
Who says this is a rhetorical question?
`-s` was introduced 18 years ago, before any of the non-patch options
were introduced.
I do not think the intention behind `-s` in 2023 is clear at all, and
the patch does not attempt to answer that.
> Unless the only effect you want is to be argumentative and annoy
> others, that is.
Assume good faith:
https://en.wikipedia.org/wiki/Wikipedia:Assume_good_faith
> I've dug the history and as I explained elsewhere in the earlier
> discussion, I know that the "--no-patch" originally was added as a
> synonym for "-s" that makes the output from the diff machinery
> silent---I have a good reason to believe that it is making "-s" and
> "--no-patch" both work as intended.
I don't think so.
`-s` might have been added to make all the diff machinery silent, but
`--no-patch` is a different question, as the commit message of d09cd15d
makes abundantly clear:
diff: allow --no-patch as synonym for -s
This follows the usual convention of having a --no-foo option to negate
--foo.
Now we know `-s` is not an antonym of `--patch`, so the commit message
of d09cd15d cannot possibly be correct.
There's only three options now:
1. `-s` doesn't turn all the diff machinery silent, only --patch
2. `--no-patch` is decoupled from `--patch`
3. `--no-patch` is decoupled from `-s`
I don't think think there's any other reasonable option, including the
status quo.
> I would not say that we should *not* move further with a follow up
> topic, but I think we should consider doing so only after the dust
> settles from this round.
But what is that dust?
Do you agree with the following?
1. No reasonable user would consider the status quo to be expected.
2. Any change to the status quo would incur in backwards-incompatible
changes for end-users.
If so, the only question remaining is what backwards-incompatible
changes shall be implemented.
--
Felipe Contreras
next prev parent reply other threads:[~2023-05-09 3:50 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
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 [this message]
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=6459c31038e81_7c68294ee@chronos.notmuch \
--to=felipe.contreras@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.
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).