Subject: Re: Can we clarify the purpose of `git diff -s`?

From: Felipe Contreras <[email-masked]>
Date: Fri, 12 May 2023 14:28:14 -0600

Junio C Hamano wrote:
> Sergey Organov <[email-masked]> writes:
> 
> > --patch. Thus, making --no-patch a synonym for -s was a mistake in the
> > first place that leaked through review process at that time, and
> >
> >    git show --format="%ad" --no-patch
> >
> > will still work the same way even if we fix --no-patch to disable
> > --patch only.
> 
> Not so fast.  I have a show.outputFormat configuration variable to
> teach builtin/log.c::show_setup_revisions_tweak() to tweak the
> hardcoded default from DIFF_FORMAT_PATCH to others (primarily
> because I often find myself doing "git show -p --stat").  Changing
> "--no-patch" to toggle only "--patch" away will close the door for
> future improvement like that, and "will still work" is an illusion.

So your rationale to reject a perfectly logical behavior that *everyone* agrees
with is that it might break a hypothetical patch?

Just do `--silent` instead.

> The user needs to be told that "--no-patch" no longer means "-s" and
> somebody needs to apologize to them that we are deliberately
> breaking their reliance they held for 10 years,

Nothing is broken.

And we never apologized for choosing the wrong default in `git pull`, did we?

> based on what we documented and prepare a smooth transition for them.

The transition is easy: just use `--silent` instead of `--no-patch`.

We could add a warning that these semantics will change in the future, instead
of just changing it right away.

But I bet very few people will see that warning (if any), because it makes
little sense to use `--no-patch` to turn off something other than `--patch`.

> Until the time when nobody uses "--no-patch" as a synonym for "-s" any
> longer, such a future improvement would be blocked.  And that is another
> reason why I want to be much more careful about "should --no-patch be changed
> to mean something other than -s" than "should -s be fixed not to be sticky
> for some but not all options".

> The latter is not a documented "feature" and it clearly was a bug that "-s
> --raw" did not honor "--raw".

Users can rely on what you call "bugs".

It's still a backwards-incompatible change for which you did not provide a
transitioning plan in [1].

Or is it a backwards-incompatible change *only* if the person proposing the
patch is somebody else other than the maintainer?

[1] https://lore.kernel.org/git/[email-masked]/

-- 
Felipe Contreras