Re: [RFC] usage_msg_opt() and _optf() must die

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2169 bytes --]

On Tue, Aug 06, 2024 at 01:21:44PM -0700, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> >> I am very much tempted to suggest us do this.
> >>
> >>  void NORETURN usage_msg_opt(const char *msg,
> >> -                  const char * const *usagestr,
> >> -                  const struct option *options)
> >> +                  const char * const *usagestr UNUSED,
> >> +                  const struct option *options UNUSED)
> >>  {
> >> -       die_message("%s\n", msg); /* The extra \n is intentional */
> >> -       usage_with_options(usagestr, options);
> >> +       die("%s", msg);
> >>  }
> >
> > As a minimal "fix" to eliminate the user-hostile behavior, I would be
> > very much in favor of this change.
> >
> > (Retiring `usage_msg_opt` altogether would be even better, but is much
> > more invasive.)
> 
> The above is following the usual "we make changes but be nice to in
> flight topics by keeping the API function still available, but the
> function now behaves better" pattern.  In other words, elimination
> of the API function is a breaking change and can go slower.  What
> needs more urgent to get to that goal would be to adjust the tests
> and documentation pages to the fallout from the above single liner.

I think it is fine to do the above as an intermediate step towards
dropping `usage_msg_opt()` altogether during this cycle. But what I'd
like to see is that we already convert all existing callers to stop
calling the function such that we can rest assured that we really can
drop the function once the Git v2.48 release cycle starts. Somewhat like
we have handled the deprecation of `struct ref_store`-less functions in
"refs.h".

Otherwise I fear that it's going to stay around indefinitely in a
misleading way.

I'm less sold on swapping the "usage:" prefix out for "error:". I think
that the "usage:" prefix actually gives a helpful signal to the user,
namely that it was the user that passed unexpected arguments. This is in
contrast to "error:", where the command went with what they gave it but
ultimately ended up running into an error condition.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]