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

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.