Re: [libvirt] [PATCH v3 06/48] remote: stop trying to print help as giant blocks of text

On 30 Jul 2019, at 12:04, Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: On Tue, Jul 30, 2019 at 11:56:45AM +0200, Christophe de Dinechin wrote: > > Daniel P. Berrangé writes: > >> The remote daemon tries to print out its help text in a couple of giant >> blocks of text. This has already lead to duplication of the text for the >> privileged vs unprivileged execution mode. With the introduction of more >> daemons, this text is going to be duplicated many more times with small >> variations. This is very unfriendly to translators as they have to >> translate approximately the same text many times with small tweaks. >> >> Splitting the text up into individual strings to print means that each >> piece will only need translating once. It also gets rid of all the >> layout information from the translated strings, so avoids the problem of >> translators breaking formatting by mistake. >> >> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; >> --- >> src/remote/remote_daemon.c | 128 ++++++++++++++++++------------------- >> src/remote/remote_driver.h | 1 - >> 2 files changed, 64 insertions(+), 65 deletions(-) >> >> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c >> index d887b7abfb..69385af1c4 100644 >> --- a/src/remote/remote_daemon.c >> +++ b/src/remote/remote_daemon.c >> @@ -859,75 +859,75 @@ daemonSetupHostUUID(const struct daemonConfig *config) >> return 0; >> } >> >> +typedef struct { >> + const char *opts; >> + const char *help; >> +} virOptionHelp; >> + >> /* Print command-line usage. */ >> static void >> daemonUsage(const char *argv0, bool privileged) >> { >> - fprintf(stderr, >> - _("\n" >> - "Usage:\n" >> - " %s [options]\n" >> - "\n" >> - "Options:\n" >> - " -h | --help Display program help:\n" >> - " -v | --verbose Verbose messages.\n" >> - " -d | --daemon Run as a daemon & write PID file.\n" >> - " -l | --listen Listen for TCP/IP connections.\n" >> - " -t | --timeout <secs> Exit after timeout period.\n" >> - " -f | --config <file> Configuration file.\n" >> - " -V | --version Display version information.\n" >> - " -p | --pid-file <file> Change name of PID file.\n" >> - "\n" >> - "libvirt management daemon:\n"), >> - argv0); >> + size_t i; >> + virOptionHelp opthelp[] = { >> + { "-h | --help", N_("Display program help") }, > > Why use N_ both here and in the printout code (copied below)? > > fprintf(stderr, " %-22s %s\n", opthelp[i].opts, N_(opthelp[i].help)); > > When is the message translated? Yeah that's a screwup. The fprintf() should be calling gettext >> + fprintf(stderr, "\n"); >> + fprintf(stderr, "%s:\n", _("Usage")); >> + fprintf(stderr, " %s [%s]\n", argv0, _("options")); > > Here, despite your argument regarding formatting, I believe that > translators should have a larger context. Also, as the gettext > documentation points out, ":" needs a space before in French, so it > should be placed within the part to translate. What documentation are you seeing this in ? I'm not come across this suggestion before and want to read more

> > fprintf(stderr, _("Usage:\n %s [options]\n\n"), argv0); > > >> + fprintf(stderr, "\n"); >> + >> + fprintf(stderr, "%s:\n", _("Options")); > > Same, for localization of whitespace around : it would be better to have > > fprintf(stderr, _("Options:\n")); > > This applies to all cases where you have %s: below. > > >> + for (i = 0; i < ARRAY_CARDINALITY(opthelp); i++) >> + fprintf(stderr, " %-22s %s\n", opthelp[i].opts, N_(opthelp[i].help)); > > Based on comment above, replace N_ with _ ? > >> + fprintf(stderr, "\n"); >> + >> + fprintf(stderr, "%s:\n", _("libvirt management daemon")); > >> + >> + fprintf(stderr, "\n"); >> + fprintf(stderr, " %s:\n", _("Default paths")); > >> + fprintf(stderr, "\n"); >> + >> + fprintf(stderr, " %s:\n", _("Configuration file (unless overridden by -f)")); > > >> + fprintf(stderr, " %s/libvirt/libvirtd.conf\n", >> + privileged ? SYSCONFDIR : "$XDG_CONFIG_HOME"); > > Add a N_ ? File paths have no translatable text in them. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list