Thread (3 messages) 3 messages, 2 authors, 2023-05-09

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

From: Junio C Hamano <hidden>
Date: 2023-05-05 16:51:43

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