Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@xxxxxxxxx> 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@xxxxxxxxxxxxxxx>
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
