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>
---
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)
}
/*
- * Print usage
+ * Build help description for commands
*/
-static void
-virshUsage(void)
+static char *
+virshBuildDescription(void)
{
const vshCmdGrp *grp;
const vshCmdDef *cmd;
-
- fprintf(stdout, _("\n%s [options]... [<command_string>]"
- "\n%s [options]... <command> [args...]\n\n"
- " options:\n"
- " -c | --connect=URI hypervisor connection
URI\n"
- " -d | --debug=NUM debug level [0-4]\n"
- " -e | --escape <char> set escape sequence for
console\n"
- " -h | --help this help\n"
- " -k | --keepalive-interval=NUM\n"
- " keepalive interval in seconds, 0
for disable\n"
- " -K | --keepalive-count=NUM\n"
- " number of possible missed
keepalive messages\n"
- " -l | --log=FILE output logging to file\n"
- " -q | --quiet quiet mode\n"
- " -r | --readonly connect readonly\n"
- " -t | --timing print timing
information\n"
- " -v short version\n"
- " -V long version\n"
- " --version[=TYPE] version, TYPE is short or long
(default short)\n"
- " commands (non interactive mode):\n\n"), progname,
- progname);
+ GString *str = g_string_new("Commands (non interactive mode):\n\n");
for (grp = cmdGroups; grp->name; grp++) {
- fprintf(stdout, _(" %s (help keyword '%s')\n"),
- grp->name, grp->keyword);
+ g_string_append_printf(str,
+ _(" %s (help keyword '%s')\n"),
+ grp->name, grp->keyword);
for (cmd = grp->commands; cmd->name; cmd++) {
if (cmd->flags & VSH_CMD_FLAG_ALIAS)
continue;
- fprintf(stdout,
- " %-30s %s\n", cmd->name,
- _(vshCmddefGetInfo(cmd, "help")));
+ g_string_append_printf(str,
+ " %-30s %s\n",
+ cmd->name,
+ _(vshCmddefGetInfo(cmd, "help")));
}
- fprintf(stdout, "\n");
+ g_string_append_printf(str, "\n");
}
- fprintf(stdout, "%s",
- _("\n (specify help <group> for details about the commands in
the group)\n"));
- fprintf(stdout, "%s",
- _("\n (specify help <command> for details about the
command)\n\n"));
- return;
+ g_string_append_printf(str,
+ _("Specify help <group> for details about the
commands in the group)\n"));
+ g_string_append_printf(str,
+ _("Specify help <command> for details about the
command)\n"));
+
+ return g_string_free(str, FALSE);
}
/*
@@ -647,6 +629,22 @@ virshAllowedEscapeChar(char c)
('@' <= c && c <= '_');
}
+static gboolean
+virshVersion(const gchar *option_name G_GNUC_UNUSED,
+ const gchar *value,
+ gpointer data,
+ GError **error G_GNUC_UNUSED)
+{
+ vshControl *ctl = data;
+
+ if (value == NULL || STRNEQ(value, "long"))
+ puts(VERSION);
+ else
+ virshShowVersion(ctl);
+
+ exit(EXIT_SUCCESS);
+}
+
/*
* argv[]: virsh [options] [command]
*
@@ -654,152 +652,121 @@ virshAllowedEscapeChar(char c)
static bool
virshParseArgv(vshControl *ctl, int argc, char **argv)
{
- int arg, len, debug, keepalive;
- size_t i;
- int longindex = -1;
+ int debug = 0;
+ bool version = false;
+ size_t len;
virshControlPtr priv = ctl->privData;
- struct option opt[] = {
- {"connect", required_argument, NULL, 'c'},
- {"debug", required_argument, NULL, 'd'},
- {"escape", required_argument, NULL, 'e'},
- {"help", no_argument, NULL, 'h'},
- {"keepalive-interval", required_argument, NULL, 'k'},
- {"keepalive-count", required_argument, NULL, 'K'},
- {"log", required_argument, NULL, 'l'},
- {"quiet", no_argument, NULL, 'q'},
- {"readonly", no_argument, NULL, 'r'},
- {"timing", no_argument, NULL, 't'},
- {"version", optional_argument, NULL, 'v'},
- {NULL, 0, NULL, 0}
+ char *logfile = NULL;
+ int keepalive_interval = INT_MAX;
+ int keepalive_count = INT_MAX;
+ GOptionEntry opt[] = {
+ { "connect", 'c', 0,
+ G_OPTION_ARG_STRING, &ctl->connname,
+ _("hypervisor connection URI"), "URI" },
+ { "debug", 'd', 0,
+ G_OPTION_ARG_INT, &debug,
+ _("debug level [0-4]\n"), "LEVEL" },
+ { "escape", 'e', 0,
+ G_OPTION_ARG_STRING, &priv->escapeChar,
+ _("set escape sequence for console"), "ESCAPE" },
+ { "keepalive-interval", 'k', 0,
+ G_OPTION_ARG_INT, &keepalive_interval,
+ _("keepalive interval in seconds, 0 for disable"), "SECS"
},
+ { "keepalive-count", 'K', 0,
+ G_OPTION_ARG_INT, &keepalive_count,
+ _("number of possible missed keepalive messages"), "NUM"
},
+ { "log", 'l', 0,
+ G_OPTION_ARG_STRING, &logfile,
+ _("output logging to file"), "FILENAME" },
+ { "quiet", 'q', 0,
+ G_OPTION_ARG_NONE, &ctl->quiet,
+ _("quite mode"), NULL },
+ { "readonly", 'r', 0,
+ G_OPTION_ARG_NONE, &priv->readonly,
+ _("connect readonly"), NULL },
+ { "timing", 't', 0,
+ G_OPTION_ARG_NONE, &ctl->timing,
+ _("print timing information"), NULL },
+ { "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.
Otherwise looks good to me.
Pavel