[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 09:56:45 UTC 2019
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?
> + { "-v | --verbose", N_("Verbose messages") },
> + { "-d | --daemon", N_("Run as a daemon & write PID file") },
> + { "-l | --listen", N_("Listen for TCP/IP connections") },
> + { "-t | --timeout <secs>", N_("Exit after timeout period") },
> + { "-f | --config <file>", N_("Configuration file") },
> + { "-V | --version", N_("Display version information") },
> + { "-p | --pid-file <file>", N_("Change name of PID file") },
> + };
>
> - if (privileged) {
> - fprintf(stderr,
> - _("\n"
> - " Default paths:\n"
> - "\n"
> - " Configuration file (unless overridden by -f):\n"
> - " %s\n"
> - "\n"
> - " Sockets:\n"
> - " %s\n"
> - " %s\n"
> - "\n"
> - " TLS:\n"
> - " CA certificate: %s\n"
> - " Server certificate: %s\n"
> - " Server private key: %s\n"
> - "\n"
> - " PID file (unless overridden by -p):\n"
> - " %s/run/libvirtd.pid\n"
> - "\n"),
> - LIBVIRTD_CONFIGURATION_FILE,
> - LIBVIRTD_PRIV_UNIX_SOCKET,
> - LIBVIRTD_PRIV_UNIX_SOCKET_RO,
> - LIBVIRT_CACERT,
> - LIBVIRT_SERVERCERT,
> - LIBVIRT_SERVERKEY,
> - LOCALSTATEDIR);
> - } else {
> - fprintf(stderr, "%s",
> - _("\n"
> - " Default paths:\n"
> - "\n"
> - " Configuration file (unless overridden by -f):\n"
> - " $XDG_CONFIG_HOME/libvirt/libvirtd.conf\n"
> - "\n"
> - " Sockets:\n"
> - " $XDG_RUNTIME_DIR/libvirt/libvirt-sock\n"
> - "\n"
> - " TLS:\n"
> - " CA certificate: $HOME/.pki/libvirt/cacert.pem\n"
> - " Server certificate: $HOME/.pki/libvirt/servercert.pem\n"
> - " Server private key: $HOME/.pki/libvirt/serverkey.pem\n"
> - "\n"
> - " PID file:\n"
> - " $XDG_RUNTIME_DIR/libvirt/libvirtd.pid\n"
> - "\n"));
> - }
> + 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.
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_ ?
> + fprintf(stderr, "\n");
> +
> + fprintf(stderr, " %s:\n", _("Sockets"));
> + fprintf(stderr, " %s\n",
> + privileged ? LOCALSTATEDIR "/run/libvirt/libvirt-sock" :
> + "$XDG_RUNTIME_DIR/libvirt/libvirt-sock");
Add N_ ?
> + if (privileged)
> + fprintf(stderr, " %s\n",
> + LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro");
Add N_?
> + fprintf(stderr, "\n");
> +
> + fprintf(stderr, " %s:\n", _("TLS"));
> + fprintf(stderr, " %s: %s\n",
> + _("CA certificate"),
> + privileged ? LIBVIRT_CACERT : "$HOME/.pki/libvirt/cacert.pem");
> + fprintf(stderr, " %s: %s\n",
> + _("Server certificate"),
> + privileged ? LIBVIRT_SERVERCERT : "$HOME/.pki/libvirt/servercert.pem");
> + fprintf(stderr, " %s: %s\n",
> + _("Server private key"),
> + privileged ? LIBVIRT_SERVERKEY : "$HOME/.pki/libvirt/serverkey.pem");
> + fprintf(stderr, "\n");
> +
> + fprintf(stderr, " %s:\n",
> + _("PID file (unless overridden by -p)"));
> + fprintf(stderr, " %s\n",
> + privileged ? LOCALSTATEDIR "/run/libvirtd.pid":
> + "$XDG_RUNTIME_DIR/libvirt/libvirtd.pid");
> + fprintf(stderr, "\n");
> }
>
> int main(int argc, char **argv) {
> diff --git a/src/remote/remote_driver.h b/src/remote/remote_driver.h
> index 8c7da6b000..132e478ef3 100644
> --- a/src/remote/remote_driver.h
> +++ b/src/remote/remote_driver.h
> @@ -34,7 +34,6 @@ unsigned long remoteVersion(void);
> #define LIBVIRTD_PRIV_UNIX_SOCKET LOCALSTATEDIR "/run/libvirt/libvirt-sock"
> #define LIBVIRTD_PRIV_UNIX_SOCKET_RO LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro"
> #define LIBVIRTD_USER_UNIX_SOCKET "libvirt-sock"
> -#define LIBVIRTD_CONFIGURATION_FILE SYSCONFDIR "/libvirt/libvirtd.conf"
>
> /* Defaults for PKI directory. */
> #define LIBVIRT_PKI_DIR SYSCONFDIR "/pki"
> --
> 2.21.0
Reviewed-by: Christophe de Dinechin <dinechin at redhat.com>
--
Cheers,
Christophe de Dinechin (IRC c3d)
More information about the libvir-list
mailing list