Re: [PATCH v2 1/4] i18n: factorize more 'incompatible options' messages

["Jean-Noël AVILA" <jn.avila@free.fr>]

Le samedi 29 janvier 2022, 09:08:34 CET Johannes Sixt a écrit :
> Am 29.01.22 um 00:58 schrieb Junio C Hamano:
> > Johannes Sixt <j6t@kdbg.org> writes:
> >> A bit of code duplication could be avoided if die_if_incompatible_opt3()
> >> forwarded with an additional pair 0, "" to die_if_incompatible_opt4().
> > 
> > I wondered if a single varargs function
> > 
> >     void die_if_incompatible_optN(const char *name1, int opt1, ...);
> > 
> > that takes a name=NULL terminated sequence of <name, opt> would
> > work, but
> > 
> >  (1) it would require the caller to always spell out the terminating
> >      NULL, which may be ugly.
> > 
> >  (2) it would tempt people to programatically generate message for N
> >      options, which leads to sentence lego, which is the exact
> >      opposite of what this series wants to achieve.

This is the second part of the functions, and they need a finite number of args 
to check in order to use the correct i18n string. This function cannot be made 
generic for any number of options to check.


> 
> The reason I did not suggest a varargs version is because
> 
>   (3) it is not typesafe.
> 
> A varargs implementation could be used as an implementation helper, but
> should not be a public interface.
> 
> > In any case, I do agree with you that the callers of _opt3()
> > variants can just pass (0, "unused") in the 4-th slot.  If this were
> > made varargs, then it only needs to pass NULL at the end, so that
> > might be an improvement, though.

I still prefer to keep a public function signature which is explicit. Using 
varargs for alternating types is really not type safe.

In such case, declaring the functions with less options as inline calls to the 
one with the greatest number is a good balance.

> > 
> > Also, isn't "if" in the name of the function misleading?  as far as
> > I can tell, this function is not "check if these options are
> > compatible and die if some are incompatible with each other", but
> > the caller is convinced that these options are incompatible before
> > it decides to call this function and there is no "if" in what this
> > function can do.
> 
> Good point.
> 

I don't get the point: the function checks if incompatible options are present 
on the command line and dies in such case. The caller does not know whether 
the function will make the program die, it really depends on the result of the 
check performed in the function. In case there's no or only one of the 
options, the function does nothing.

JN