Re: [libvirt] [PATCHv2 11/15] virsh: add option aliases
On 03/06/2012 01:34 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. --- As a side note: It would be the best with this change to put a warning into the mang page that options that disappeared from the help/man page are not vanished but are obsolete and the user should use the new spellings but we still support the legacy ones. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 11/15] virsh: add option aliases
On 03/06/2012 10:46 AM, Peter Krempa wrote: On 03/06/2012 01:34 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. --- Nice way to mask old mistakes and still support them. I'm wondering if this will not confuse people if their beloved arguments disappear suddenly from the docs. Maybe the help command could explicitly state aliases that exist for commands to avoid some confusion. I'll document the old name in the man page, but not in 'virsh help'. I'm leaning towards an ACK as it's better to encourage to use the fixed spelling. Does anyone object? Peter -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 11/15] virsh: add option aliases
On 03/06/2012 01:34 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. --- Nice way to mask old mistakes and still support them. I'm wondering if this will not confuse people if their beloved arguments disappear suddenly from the docs. Maybe the help command could explicitly state aliases that exist for commands to avoid some confusion. I'm leaning towards an ACK as it's better to encourage to use the fixed spelling. Does anyone object? Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 11/15] virsh: add option aliases
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.
---
v2: no real change
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);
}
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
