Subject: Re: [PATCH] diff: fix behaviour of the "-s" option

From: Junio C Hamano <[email-masked]>
Date: Fri, 05 May 2023 09:51:38 -0700

Junio C Hamano <[email-masked]> writes:

> ...  I think the "historical
> reasons" why we did not do that was because we wanted to be able to
> do a flexible defaulting. ...
> This would almost work, except that it would
> make it hard to tell "no command line options" case and "'-s' cleared
> all bits" case apart (the former wants to default to "--patch",
> while the latter wants to stay "no output"), and it probably was the
> reason why we gave an extra NO_OUTPUT bit to the "-s" option.  In
> hindsight, the arrangement certainly made other things harder and
> prone to unnecessary bugs.
>
> Anyway...

The distinction between the presense of NO_OUTPUT bit and absolutely
empty output_format word indeed is used by "git show", in the
builtin/log.c::show_setup_revisions_tweak() function.

We could lose DIFF_FORMAT_NO_OUTPUT bit, but then we need to replace
it with something else (i.e. DIFF_FORMAT_OPTION_GIVEN bit), and

 * "--patch", "--raw", etc. will set DIFF_FORMAT_$format bit and
   DIFF_FORMAT_OPTION_GIVEN bit on for each format.  "--no-raw", 
   etc. will set off DIFF_FORMAT_$format bit but still record the
   fact that we saw an option from the command line by setting
   DIFF_FORMAT_OPTION_GIVEN bit.

 * "-s" (and its synonym "--no-patch") will set the
   DIFF_FORMAT_OPTION_GIVEN bit on, and clear all other bits.

which I suspect would make the code much cleaner without breaking
any end-user expectations.

Once that is in place, transitioning "--no-patch" to mean the
counterpart of "--patch", just like "--no-raw" only defeats an
earlier "--raw", would be quite simple at the code level.  The
social cost of migrating the end-user expectations might be too
great for it to be worth, but at least the "GIVEN" bit clean-up
alone may be worth it.

Not that I would be starting the process right away...