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@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