[libvirt] [PATCH 1/3] virsh: add option aliases

[Osier Yang]

On 03/03/2012 09:02 AM, Eric Blake wrote:
> In the past, we have created some virsh options with less-than-stellar
> names.  For back-compat reasons, those names must continue to parse,
> but we don't want to document them in help output.  This introduces
> a new option type, an alias, which points to a canonical option name
> later in the option list.
>
> I'm actually quite impressed that our code has already been factored
> to do all option parsing through common entry points, such that I
> got this added in relatively few lines of code!
>
> * tools/virsh.c (VSH_OT_ALIAS): New option type.
> (opts_echo): Hook up an alias, for easy testing.
> (vshCmddefOptParse, vshCmddefHelp, vshCmddefGetOption): Allow for
> aliases.
> * tests/virshtest.c (mymain): Test new feature.
> ---
>   tests/virshtest.c |    6 ++++++
>   tools/virsh.c     |   28 ++++++++++++++++++++++++++--
>   2 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/tests/virshtest.c b/tests/virshtest.c
> index 6474c19..87b1336 100644
> --- a/tests/virshtest.c
> +++ b/tests/virshtest.c
> @@ -386,6 +386,12 @@ mymain(void)
>       DO_TEST(30, "--shell a\n",
>               "echo \t '-'\"-\" \t --shell \t a");
>
> +    /* Tests of alias handling.  */
> +    DO_TEST(31, "hello\n", "echo", "--string", "hello");
> +    DO_TEST(32, "hello\n", "echo --string hello");
> +    DO_TEST(33, "hello\n", "echo", "--str", "hello");
> +    DO_TEST(34, "hello\n", "echo --str hello");
> +
>   # undef DO_TEST
>
>       VIR_FREE(custom_uri);
> diff --git a/tools/virsh.c b/tools/virsh.c
> index aef050f..77cf4ac 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -138,7 +138,8 @@ typedef enum {
>       VSH_OT_STRING,   /* optional string option */
>       VSH_OT_INT,      /* optional or mandatory int option */
>       VSH_OT_DATA,     /* string data (as non-option) */
> -    VSH_OT_ARGV      /* remaining arguments */
> +    VSH_OT_ARGV,     /* remaining arguments */
> +    VSH_OT_ALIAS,    /* alternate spelling for a later argument */
>   } vshCmdOptType;
>
>   /*
> @@ -190,7 +191,8 @@ typedef struct {
>       const char *name;           /* the name of option, or NULL for list end */
>       vshCmdOptType type;         /* option type */
>       unsigned int flags;         /* flags */
> -    const char *help;           /* non-NULL help string */
> +    const char *help;           /* non-NULL help string; or for VSH_OT_ALIAS
> +                                 * the name of a later public option */
>   } vshCmdOptDef;
>
>   /*
> @@ -15209,6 +15211,7 @@ static const vshCmdInfo info_echo[] = {
>   static const vshCmdOptDef opts_echo[] = {
>       {"shell", VSH_OT_BOOL, 0, N_("escape for shell use")},
>       {"xml", VSH_OT_BOOL, 0, N_("escape for XML use")},
> +    {"str", VSH_OT_ALIAS, 0, "string"},
>       {"string", VSH_OT_ARGV, 0, N_("arguments to echo")},
>       {NULL, 0, 0, NULL}
>   };
> @@ -17256,6 +17259,18 @@ vshCmddefOptParse(const vshCmdDef *cmd, uint32_t *opts_need_arg,
>                   return -1; /* bool options can't be mandatory */
>               continue;
>           }
> +        if (opt->type == VSH_OT_ALIAS) {
> +            int j;
> +            if (opt->flags || !opt->help)
> +                return -1; /* alias options are tracked by the original name */
> +            for (j = i + 1; cmd->opts[j].name; j++) {
> +                if (STREQ(opt->help, cmd->opts[j].name))
> +                    break;
> +            }
> +            if (!cmd->opts[j].name)
> +                return -1; /* alias option must map to a later option name */
> +            continue;
> +        }
>           if (opt->flags&  VSH_OFLAG_REQ_OPT) {
>               if (opt->flags&  VSH_OFLAG_REQ)
>                   *opts_required |= 1<<  i;
> @@ -17287,6 +17302,10 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name,
>           const vshCmdOptDef *opt =&cmd->opts[i];
>
>           if (STREQ(opt->name, name)) {
> +            if (opt->type == VSH_OT_ALIAS) {
> +                name = opt->help;
> +                continue;
> +            }
>               if ((*opts_seen&  (1<<  i))&&  opt->type != VSH_OT_ARGV) {
>                   vshError(ctl, _("option --%s already seen"), name);
>                   return NULL;
> @@ -17465,6 +17484,9 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
>                               : _("[<%s>]...");
>                       }
>                       break;
> +                case VSH_OT_ALIAS:
> +                    /* aliases are intentionally undocumented */
> +                    continue;
>                   default:
>                       assert(0);
>                   }
> @@ -17506,6 +17528,8 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
>                                shortopt ? _("[--%s]<string>") : _("<%s>"),
>                                opt->name);
>                       break;
> +                case VSH_OT_ALIAS:
> +                    continue;
>                   default:
>                       assert(0);
>                   }

Ah, I could recall we talked about this half year before, for
creating alias ("--config") for the options "--persistent" of
commands like "attach-device", then I forgot it to do it.

Looks good, ACK.

Osier