| Message ID | 20220907142359.31827-1-cfontana@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | audio: add help option (?) for -audiodev | expand |
Claudio Fontana <cfontana@suse.de> writes: > add a simple help option for -audiodev, so users can do > > qemu -audiodev ? The preferred form is actually '-audiodev help'. The other one is deprecated. Recommend to stay away from it even in commit messages. > > to get the list of drivers available. > > Signed-off-by: Claudio Fontana <cfontana@suse.de> > --- > audio/audio.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/audio/audio.c b/audio/audio.c > index 4f4bb10cce..bd8c18c3cd 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -32,6 +32,7 @@ > #include "qapi/qapi-visit-audio.h" > #include "qemu/cutils.h" > #include "qemu/module.h" > +#include "qemu/help_option.h" > #include "sysemu/sysemu.h" > #include "sysemu/replay.h" > #include "sysemu/runstate.h" > @@ -2105,10 +2106,29 @@ static void audio_validate_opts(Audiodev *dev, Error **errp) > } > } > > +static void audio_help(void) > +{ > + int i; > + > + printf("Available audiodev types:\n"); > + printf("none\n"); > + > + for (i = 0; audio_prio_list[i]; i++) { > + audio_driver *driver = audio_driver_lookup(audio_prio_list[i]); > + if (driver) { > + printf("%s\n", driver->name); > + } > + } > +} > + > void audio_parse_option(const char *opt) > { > Audiodev *dev = NULL; > > + if (is_help_option(opt)) { > + audio_help(); > + exit(0); > + } > Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal); > visit_type_Audiodev(v, NULL, &dev, &error_fatal); > visit_free(v);
On Wed, Sep 07, 2022 at 05:06:36PM +0200, Markus Armbruster wrote: > Claudio Fontana <cfontana@suse.de> writes: > > > add a simple help option for -audiodev, so users can do > > > > qemu -audiodev ? > > The preferred form is actually '-audiodev help'. The other one is > deprecated. Recommend to stay away from it even in commit messages. We introduced 'help' many many years ago, but don't thing we ever formally deprecated '?'. Should we do so and aim to remove it, or are we happy to keep '?' forever, despite it tripping up shell filename expansion with single char filenames. (tangential to this patch, NOT a request to fix something in v2) > > > > to get the list of drivers available. > > > > Signed-off-by: Claudio Fontana <cfontana@suse.de> > > --- > > audio/audio.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) Update qemu-options.hx ? With regards, Daniel
On 9/7/22 17:06, Markus Armbruster wrote: > Claudio Fontana <cfontana@suse.de> writes: > >> add a simple help option for -audiodev, so users can do >> >> qemu -audiodev ? > > The preferred form is actually '-audiodev help'. The other one is > deprecated. Recommend to stay away from it even in commit messages. I have no problem mentioning "help" instead of "?" in the commit message (or both). The function used (is_help_option) checks for both, which seems the best way to me. Ciao C > >> >> to get the list of drivers available. >> >> Signed-off-by: Claudio Fontana <cfontana@suse.de> >> --- >> audio/audio.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/audio/audio.c b/audio/audio.c >> index 4f4bb10cce..bd8c18c3cd 100644 >> --- a/audio/audio.c >> +++ b/audio/audio.c >> @@ -32,6 +32,7 @@ >> #include "qapi/qapi-visit-audio.h" >> #include "qemu/cutils.h" >> #include "qemu/module.h" >> +#include "qemu/help_option.h" >> #include "sysemu/sysemu.h" >> #include "sysemu/replay.h" >> #include "sysemu/runstate.h" >> @@ -2105,10 +2106,29 @@ static void audio_validate_opts(Audiodev *dev, Error **errp) >> } >> } >> >> +static void audio_help(void) >> +{ >> + int i; >> + >> + printf("Available audiodev types:\n"); >> + printf("none\n"); >> + >> + for (i = 0; audio_prio_list[i]; i++) { >> + audio_driver *driver = audio_driver_lookup(audio_prio_list[i]); >> + if (driver) { >> + printf("%s\n", driver->name); >> + } >> + } >> +} >> + >> void audio_parse_option(const char *opt) >> { >> Audiodev *dev = NULL; >> >> + if (is_help_option(opt)) { >> + audio_help(); >> + exit(0); >> + } >> Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal); >> visit_type_Audiodev(v, NULL, &dev, &error_fatal); >> visit_free(v); >
On Wed, 7 Sep 2022, Daniel P. Berrangé wrote: > On Wed, Sep 07, 2022 at 05:06:36PM +0200, Markus Armbruster wrote: >> Claudio Fontana <cfontana@suse.de> writes: >> >>> add a simple help option for -audiodev, so users can do >>> >>> qemu -audiodev ? >> >> The preferred form is actually '-audiodev help'. The other one is >> deprecated. Recommend to stay away from it even in commit messages. > > We introduced 'help' many many years ago, but don't thing we > ever formally deprecated '?'. Should we do so and aim to > remove it, or are we happy to keep '?' forever, despite it > tripping up shell filename expansion with single char filenames. Had this conversation before and I think we agreed to keep ? as a convenient shorthand even if help is preferred so it should not be deprecated but you can keep it from appearing in docs to advertise help as the preferred option. (\? is still shorter to type than help if you're worried about shell expansion which is rarely a problem in practice) Regards, BALATON Zoltan > (tangential to this patch, NOT a request to fix something in v2) > >>> >>> to get the list of drivers available. >>> >>> Signed-off-by: Claudio Fontana <cfontana@suse.de> >>> --- >>> audio/audio.c | 20 ++++++++++++++++++++ >>> 1 file changed, 20 insertions(+) > > Update qemu-options.hx ? > > > With regards, > Daniel >
Daniel P. Berrangé <berrange@redhat.com> writes: > On Wed, Sep 07, 2022 at 05:06:36PM +0200, Markus Armbruster wrote: >> Claudio Fontana <cfontana@suse.de> writes: >> >> > add a simple help option for -audiodev, so users can do >> > >> > qemu -audiodev ? >> >> The preferred form is actually '-audiodev help'. The other one is >> deprecated. Recommend to stay away from it even in commit messages. > > We introduced 'help' many many years ago, but don't thing we > ever formally deprecated '?'. is_help_option()'s function comment says "'?' is deprecated". Goes back to commit c8057f951d64de93bfd01569c0a725baa9f94372 Author: Peter Maydell <peter.maydell@linaro.org> Date: Thu Aug 2 13:45:54 2012 +0100 Support 'help' as a synonym for '?' in command line options For command line options which permit '?' meaning 'please list the permitted values', add support for 'help' as a synonym, by abstracting the check out into a helper function. This change means that in some cases where we were being lazy in our string parsing, "?junk" will now be rejected as an invalid option rather than being (undocumentedly) treated the same way as "?". Update the documentation to use 'help' rather than '?', since '?' is a shell metacharacter and thus prone to fail confusingly if there is a single character filename in the current working directory and the '?' has not been escaped. It's therefore better to steer users towards 'help', though '?' is retained for backwards compatibility. We do not, however, update the output of the system emulator's -help (or any documentation autogenerated from the qemu-options.hx which is the source of the -help text) because libvirt parses our -help output and will break. At a later date when QEMU provides a better interface so libvirt can avoid having to do this, we can update the -help text too. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> The more formal deprecation process we use today didn't exist in 2012. Looks like we simply de-documented '?'. Note that output of -help has been updated since. > Should we do so and aim to > remove it, or are we happy to keep '?' forever, despite it > tripping up shell filename expansion with single char filenames. I'm not sure a belated formal notice of deprecation would be useful. Emitting a warning might be. Would have to be done at roughly 20 call sites, I guess. I'm cool with removing a feature that has been undocumented for a decade. However, experience tells us that every feature on the chopping block will find a champion, and I believe this one is still around simply because it's not worth a fight. [...]
diff --git a/audio/audio.c b/audio/audio.c index 4f4bb10cce..bd8c18c3cd 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -32,6 +32,7 @@ #include "qapi/qapi-visit-audio.h" #include "qemu/cutils.h" #include "qemu/module.h" +#include "qemu/help_option.h" #include "sysemu/sysemu.h" #include "sysemu/replay.h" #include "sysemu/runstate.h" @@ -2105,10 +2106,29 @@ static void audio_validate_opts(Audiodev *dev, Error **errp) } } +static void audio_help(void) +{ + int i; + + printf("Available audiodev types:\n"); + printf("none\n"); + + for (i = 0; audio_prio_list[i]; i++) { + audio_driver *driver = audio_driver_lookup(audio_prio_list[i]); + if (driver) { + printf("%s\n", driver->name); + } + } +} + void audio_parse_option(const char *opt) { Audiodev *dev = NULL; + if (is_help_option(opt)) { + audio_help(); + exit(0); + } Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal); visit_type_Audiodev(v, NULL, &dev, &error_fatal); visit_free(v);
add a simple help option for -audiodev, so users can do qemu -audiodev ? to get the list of drivers available. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- audio/audio.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)