diff mbox

[v3] Support 'help' as a synonym for '?' in command line options

Message ID CAFEAcA-3AertMzzpF=jdC__7QAa2RKneS3OitqNDJwXetRbiPA@mail.gmail.com
State Superseded
Headers show

Commit Message

Peter Maydell Aug. 9, 2012, 9:02 p.m. UTC
On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > 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.

>> Applied. Thanks.
>
> I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
> "-cpu ?model":

These options appear to be completely undocumented. They're also pretty
ugly syntax and seem to be x86 specific. However we can unbreak them
if we must with a patch like this:


(will send as a proper patch with commit message and signoff tomorrow).

Any suggestions for what the sane syntax for these options would be?
(ie the analogous change to having '?' go to 'help').

-- PMM

Comments

Eric Blake Aug. 9, 2012, 9:56 p.m. UTC | #1
On 08/09/2012 03:02 PM, Peter Maydell wrote:
> On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>> 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.
> 
>>> Applied. Thanks.
>>
>> I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
>> "-cpu ?model":
> 
> These options appear to be completely undocumented. They're also pretty
> ugly syntax and seem to be x86 specific.

Thankfully, libvirt is not using them.  If there is a reason for libvirt
to start using them, then libvirt would rather go through QMP than any
new syntax when talking to newer qemu (that is, I'm okay if we
completely lose the spelling as long as we don't lose the feature).

> 
> Any suggestions for what the sane syntax for these options would be?
> (ie the analogous change to having '?' go to 'help').

-cpu help=dump
-cpu help=cpuid
Eduardo Habkost Aug. 9, 2012, 9:58 p.m. UTC | #2
On Thu, Aug 09, 2012 at 10:02:22PM +0100, Peter Maydell wrote:
> On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >> > 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.
> 
> >> Applied. Thanks.
> >
> > I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
> > "-cpu ?model":
> 
> These options appear to be completely undocumented. They're also pretty
> ugly syntax and seem to be x86 specific.

Agreed. I wasn't aware it was completely undocumented, I thought there
was documentation somewhere.


> However we can unbreak them
> if we must with a patch like this:
> 
> --- a/vl.c
> +++ b/vl.c
> @@ -3215,7 +3215,11 @@ int main(int argc, char **argv, char **envp)
>       */
>      cpudef_init();
> 
> -    if (cpu_model && is_help_option(cpu_model)) {
> +    /* We have to check for "starts with '?' as well as is_help_option
> +     * to support targets which implement various weird help options
> +     * via '?thingy' syntax.
> +     */
> +    if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) {
>          list_cpus(stdout, &fprintf, cpu_model);
>          exit(0);
>      }
> 
> (will send as a proper patch with commit message and signoff tomorrow).
> 
> Any suggestions for what the sane syntax for these options would be?
> (ie the analogous change to having '?' go to 'help').

What about "-cpu help,dump" or "-cpu help=dump"?
Anthony Liguori Aug. 10, 2012, 1:45 p.m. UTC | #3
Eduardo Habkost <ehabkost@redhat.com> writes:

> On Thu, Aug 09, 2012 at 10:02:22PM +0100, Peter Maydell wrote:
>> On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> > On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
>> >> Peter Maydell <peter.maydell@linaro.org> writes:
>> >> > 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.
>> 
>> >> Applied. Thanks.
>> >
>> > I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
>> > "-cpu ?model":
>> 
>> These options appear to be completely undocumented. They're also pretty
>> ugly syntax and seem to be x86 specific.
>
> Agreed. I wasn't aware it was completely undocumented, I thought there
> was documentation somewhere.
>
>
>> However we can unbreak them
>> if we must with a patch like this:
>> 
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3215,7 +3215,11 @@ int main(int argc, char **argv, char **envp)
>>       */
>>      cpudef_init();
>> 
>> -    if (cpu_model && is_help_option(cpu_model)) {
>> +    /* We have to check for "starts with '?' as well as is_help_option
>> +     * to support targets which implement various weird help options
>> +     * via '?thingy' syntax.
>> +     */
>> +    if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) {
>>          list_cpus(stdout, &fprintf, cpu_model);
>>          exit(0);
>>      }
>> 
>> (will send as a proper patch with commit message and signoff tomorrow).
>> 
>> Any suggestions for what the sane syntax for these options would be?
>> (ie the analogous change to having '?' go to 'help').
>
> What about "-cpu help,dump" or "-cpu help=dump"?

Let's just drop the feature.  I doubt a user would ever do this.

For 1.3, I'd like to introduce glib option groups and allow for group
specific help options.  IOW, --help-cpu

Regards,

Anthony Liguori

>
> -- 
> Eduardo
diff mbox

Patch

--- a/vl.c
+++ b/vl.c
@@ -3215,7 +3215,11 @@  int main(int argc, char **argv, char **envp)
      */
     cpudef_init();

-    if (cpu_model && is_help_option(cpu_model)) {
+    /* We have to check for "starts with '?' as well as is_help_option
+     * to support targets which implement various weird help options
+     * via '?thingy' syntax.
+     */
+    if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) {
         list_cpus(stdout, &fprintf, cpu_model);
         exit(0);
     }