On Wed, Jun 28, 2017 at 09:29:40PM +0200, Ruediger Meier wrote: > On Tuesday 20 June 2017, Karel Zak wrote: > > On Sun, Jun 18, 2017 at 08:49:49PM -0400, J William Piggott wrote: > > > sys-utils/hwclock.c | 74 > > > ++++++++++++++++++++++++++--------------------------- 1 file > > > changed, 37 insertions(+), 37 deletions(-) > > > > It would be better to remove this patch from the pull-request; let's > > keep fputs() in the code and wait for any solution from Rudi :-) > > I have now finished my cleanup regarding stdout only. To make the diffs > small I have left a useless definition "FILE *out = stdout;" in almost > any usage function. > > We can remove this "out" variable now everwhere using a sed or awk. But > a few questions about what would be the best end state. > > 1. fputs vs puts?: > > Is it a problem for translators if we remove a newline from almost > any string? And, does puts() look more nice at all? I mean many usage > functions have to use printf too, so does it look good if some > strings are '\n' terminated and others not? Frankly, I prefer to have \n in the string, because in this case you have full control on the output. And it also means all the strings modification... for me fputs() is the winner :-) > 2. Alignment for better readabilty in the code. > > I would prefer leading spaces before the first string argument to > match the longest possible prefix 'printf(_("'. Is that ok?: > > puts( _(" -q quiet mode")); > fputs( _(" -v, --verbose verbose mode"), stdout); > printf(_(" -f, --rtc use alternate to %1$s\n"), _BLA); > printf( " --help %s\n", USAGE_OPTSTR_HELP); Yes, good idea. > 3. Maybe we should always decouple options and descriptions? > > Introducing a macro like: > > #define prnt_opt(opt, marg_dsc, dsc) \ > printf( "%-" #marg_dsc "s%s\n", opt, dscr) > > /* the magic margin number for the whole function */ > int m = 16; > prnt_opt(" -q", m, _("quiet mode"); > prnt_opt(" -v, --verbose", m, _("verbose mode"); > prnt_opt(" --help", m, USAGE_OPTSTR_HELP); > /* or standard help, as exist already */ > print_usage_help_options(m); Hmm... now translator can also translate option arguments --something <time> this is nice option in this case <time> is possible to translate too, and maybe in some languages it's possible that you will need to change number of blank spaces between option and option description. IMHO it's against KISS principle :-) > 4. About our USAGE_* macros > > We can remove the trailing newlines in all macros if puts() is > preferred (regarding 1.). Or we can just let them print like > print_usage_help_options(). Frankly, for me it's more readable to have fputs(USAGE_OPTIONS, stdout); than introduce another function. (It was also reason why I was not sure with print_usage_help_options(), but it resolves more issues.) > 5. We can leave everything as is and touch all these usage lines only > if needed. For example I did it without extra noise in a861538c. > Karel missed his chance in 4fb515f9 ;) It was my goal not to do any change if you work in this area. And yes, "touch only if needed" is good idea. The blockdev usage() is nice example that mix more things together is no problem, it's still readable and without extra complexity (well, maybe use COMMANDS: there :-). > Note since introducing print_usage_help_options() we *are* already > mixing hardcoded stdout and variable out. So we can also convert > single lines from fprintf to printf, like i did in my last pending > hwclock patch "don't ifdef printf arguments". All the issue is libc stupidity, imagine world where puts() is without \n junk (like printf and fprintf). I think printf() is no problem there. For me it's about: * nice usage() output * usage() function readability * all without extra complexity (random contributor has to be able add new option easily) * translators-friendly (minimize changes, keep it open as much as possible) Karel -- Karel Zak <kzak@xxxxxxxxxx> http://karelzak.blogspot.com -- To unsubscribe from this list: send the line "unsubscribe util-linux" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html