On 06/11/2018 02:33, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes: > >> There are a few issues with opterror() >> >> - it tries to assemble an English sentence from pieces. This is not >> great for translators because we give them pieces instead of a full >> sentence. >> >> - It's a wrapper around error() and needs some hack to let the >> compiler know it always returns -1. >> >> - Since it takes a string instead of printf format, one call site has >> to assemble the string manually before passing to it. >> >> Kill it and produce the option name with optname(). The user will use >> error() directly. This solves the second and third problems. > > The proposed log message is not very friendly to reviewers, as there > is no hint what optname() does nor where it came from; it turns out > that this patch introduces it. > > Introduce optname() that does the early half of original > opterror() to come up with the name of the option reported back > to the user, and use it to kill opterror(). The callers of > opterror() now directly call error() using the string returned > by opterror() instead. > > or something like that perhaps. > > Theoretically not very friendly to topics in flight, but I do not > expect there would be any right now that wants to add new callers of > opterror(). > > I do agree with the reasoning behind this change. Thanks for > working on it. > Also, this patch does not replace opterror() calls outside of the 'parse-options.c' file with optname(). This tickles my static-check.pl script, since optname() is an external function which is only called from 'parse-options.c'. So, at present, optname() could be marked as a local 'static' symbol. However, I could also imagine it being used by new callers outside of 'parse-options.c' in the future. (maybe) Your call. ;-) ATB, Ramsay Jones
