Thread (7 messages) 7 messages, 3 authors, 2023-05-13

Re: [PATCH v2] diff: fix interaction between the "-s" option and other options

From: Felipe Contreras <hidden>
Date: 2023-05-09 03:50:45

Junio C Hamano wrote:
Felipe Contreras [off-list ref] writes:
quoted
quoted
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 [off-list ref]
  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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help