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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 26, 2019 at 02:02:18PM +0200, Andrea Bolognani wrote:
> On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
> [...]
> > +    struct virOptionHelp {
> > +        const char *opts;
> > +        const char *help;
> > +    } opthelp[] = {
> > +        { "-h | --help", N_("Display program help") },
> > +        { "-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") },
> > +    };
> 
> The way you declare the struct at the same time as you use it for a
> local variable is highly unusual in our codebase, especially
> considering that you do the former inside a function which is, at
> least as far as I'm aware, basically unprecedented in libvirt.
> 
> Can you please move the struct declaration outside of the function?
> 
> [...]
> > +    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");
> 
> I think the above would work better if you used
> 
>   "      %-18s  %s\n"
> 
> as the format string, which would result in
> 
>   TLS:
>     CA certificate      $HOME/.pki/libvirt/cacert.pem
>     Server certificate  $HOME/.pki/libvirt/servercert.pem
>     Server private key  $HOME/.pki/libvirt/serverkey.pem
> 
> instead of
> 
>   TLS:
>     CA certificate: $HOME/.pki/libvirt/cacert.pem
>     Server certificate: $HOME/.pki/libvirt/servercert.pem
>     Server private key: $HOME/.pki/libvirt/serverkey.pem

You're only thinking about this from POV of english text. When this
is translated into other languages, all bets are off for such
alignment, as translated text could easily be longer than 18 characters
and I don't fancy guessing the max field width to cope with arbitrary
languages.

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux