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

Christophe de Dinechin dinechin at redhat.com
Tue Jul 30 10:10:23 UTC 2019



> On 30 Jul 2019, at 12:04, Daniel P. Berrangé <berrange at redhat.com> 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 at redhat.com>
>>> ---
>>> 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.

Well, I’ve always been confused with what “N_” meant exactly, but
to me it was a tell to translator “don’t try translating this one”.
If there is nothing, the translator might waste time looking at it.
Maybe I got that backwards, though. Can’t say the doc is super clear.

> 
> 
> 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list





More information about the libvir-list mailing list