Re: [libvirt] [PATCH 09/11] virsh: convert command line parsing to use GOptionContext

On Tue, Oct 01, 2019 at 11:37:17AM +0200, Pavel Hrdina wrote: > On Fri, Sep 27, 2019 at 06:17:31PM +0100, Daniel P. Berrangé wrote: > > The GOptionContext API has the benefit over getopt_long that it will > > automatically handle --help output formatting. > > > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > > --- > > tools/virsh.c | 303 ++++++++++++++++++++++---------------------------- > > 1 file changed, 135 insertions(+), 168 deletions(-) > > > > diff --git a/tools/virsh.c b/tools/virsh.c > > index ec20f35a77..6c469ff576 100644 > > --- a/tools/virsh.c > > +++ b/tools/virsh.c > > @@ -23,7 +23,6 @@ > > > > #include <stdarg.h> > > #include <unistd.h> > > -#include <getopt.h> > > #include <sys/time.h> > > #include <fcntl.h> > > #include <time.h> > > @@ -445,53 +444,36 @@ virshDeinit(vshControl *ctl) > > } > > + { "version", 'v', G_OPTION_FLAG_OPTIONAL_ARG, > > + G_OPTION_ARG_CALLBACK, virshVersion, > > + _("print short version"), "[short]" }, > > + { "version", 'V', 0, > > + G_OPTION_ARG_NONE, &version, > > + _("print long version"), "long" }, > > We should be able to have both -v and -V call virshVersion if the > functions will look like this: > > static gboolean > virshVersion(const gchar *option_name, > const gchar *value, > gpointer data, > GError **error G_GNUC_UNUSED) > { > vshControl *ctl = data; > > if (STREQ(option_name, "-V") || STREQ_NULLABLE(value, "long")) > virshShowVersion(ctl); > else > puts(VERSION); > > exit(EXIT_SUCCESS); > } > > That way we will have only a single place where the version printing > code is. Hmm, so "option_name" in the callback is the option that the user actually passed ? I was thinking it was the option name from the static array - ie it would always be "version". If you're right though, this is definitely my preference.