| Message ID | 1532012617-16777-1-git-send-email-walling@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
You neglected to cc: maintainers. Cc'ing them increases the odds your patch will be noticed and picked up. You can use scripts/get_maintainer.pl to find maintainers. You don't have to do anything for this patch; it got noticed anyway. David, this is yours :) Collin Walling <walling@linux.ibm.com> writes: > When typing 'help' followed by an unknown command, QEMU will > not print anything to the command line to let the user know > they typed a bad command. Let's fix this by printing a message > to the monitor when this happens. For example: > > (qemu) help xyz > unknown command: 'xyz' > > Reported-by: Stefan Zimmermann <stzi@linux.ibm.com> > Signed-off-by: Collin Walling <walling@linux.ibm.com> > --- > monitor.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/monitor.c b/monitor.c > index 7af1f18..7942f9f 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1034,9 +1034,12 @@ static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds, > } else { > help_cmd_dump_one(mon, cmd, args, arg_index); > } > - break; > + return; > } > } > + > + /* Entry not found */ > + monitor_printf(mon, "unknown command: '%s'\n", args[arg_index]); > } > > static void help_cmd(Monitor *mon, const char *name)
On 07/19/2018 12:31 PM, Markus Armbruster wrote: > You neglected to cc: maintainers. Cc'ing them increases the odds your > patch will be noticed and picked up. You can use > scripts/get_maintainer.pl to find maintainers. You don't have to do > anything for this patch; it got noticed anyway. > > David, this is yours :) Very true. Was a minor fix that I thought I'd just toss it out there and let anyone view it if they had the time. Will be more aware of who to CC next time around. Thanks :) > > Collin Walling <walling@linux.ibm.com> writes: > >> When typing 'help' followed by an unknown command, QEMU will >> not print anything to the command line to let the user know >> they typed a bad command. Let's fix this by printing a message >> to the monitor when this happens. For example: >> >> (qemu) help xyz >> unknown command: 'xyz' >> >> Reported-by: Stefan Zimmermann <stzi@linux.ibm.com> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> >> --- >> monitor.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/monitor.c b/monitor.c >> index 7af1f18..7942f9f 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -1034,9 +1034,12 @@ static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds, >> } else { >> help_cmd_dump_one(mon, cmd, args, arg_index); >> } >> - break; >> + return; >> } >> } >> + >> + /* Entry not found */ >> + monitor_printf(mon, "unknown command: '%s'\n", args[arg_index]); >> } >> >> static void help_cmd(Monitor *mon, const char *name) >
* Markus Armbruster (armbru@redhat.com) wrote: > You neglected to cc: maintainers. Cc'ing them increases the odds your > patch will be noticed and picked up. You can use > scripts/get_maintainer.pl to find maintainers. You don't have to do > anything for this patch; it got noticed anyway. > > David, this is yours :) Thanks! Dave > Collin Walling <walling@linux.ibm.com> writes: > > > When typing 'help' followed by an unknown command, QEMU will > > not print anything to the command line to let the user know > > they typed a bad command. Let's fix this by printing a message > > to the monitor when this happens. For example: > > > > (qemu) help xyz > > unknown command: 'xyz' > > > > Reported-by: Stefan Zimmermann <stzi@linux.ibm.com> > > Signed-off-by: Collin Walling <walling@linux.ibm.com> > > --- > > monitor.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/monitor.c b/monitor.c > > index 7af1f18..7942f9f 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -1034,9 +1034,12 @@ static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds, > > } else { > > help_cmd_dump_one(mon, cmd, args, arg_index); > > } > > - break; > > + return; > > } > > } > > + > > + /* Entry not found */ > > + monitor_printf(mon, "unknown command: '%s'\n", args[arg_index]); > > } > > > > static void help_cmd(Monitor *mon, const char *name) -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Collin Walling (walling@linux.ibm.com) wrote: > When typing 'help' followed by an unknown command, QEMU will > not print anything to the command line to let the user know > they typed a bad command. Let's fix this by printing a message > to the monitor when this happens. For example: > > (qemu) help xyz > unknown command: 'xyz' > > Reported-by: Stefan Zimmermann <stzi@linux.ibm.com> > Signed-off-by: Collin Walling <walling@linux.ibm.com> > --- > monitor.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/monitor.c b/monitor.c > index 7af1f18..7942f9f 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1034,9 +1034,12 @@ static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds, > } else { > help_cmd_dump_one(mon, cmd, args, arg_index); > } > - break; > + return; > } > } > + > + /* Entry not found */ > + monitor_printf(mon, "unknown command: '%s'\n", args[arg_index]); Thanks, that does suffer from a similar bug to the one you fixed a few months back in 317c52cc6aa0d ('monitor: report entirety of hmp command on error'): (qemu) help foo unknown command: 'foo' (qemu) help info foo unknown command: 'foo' Dave (And yes, please cc me, otherwise I can miss them) > } > > static void help_cmd(Monitor *mon, const char *name) > -- > 2.7.4 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 07/19/2018 03:18 PM, Dr. David Alan Gilbert wrote: > * Collin Walling (walling@linux.ibm.com) wrote: >> When typing 'help' followed by an unknown command, QEMU will >> not print anything to the command line to let the user know >> they typed a bad command. Let's fix this by printing a message >> to the monitor when this happens. For example: >> >> (qemu) help xyz >> unknown command: 'xyz' >> >> Reported-by: Stefan Zimmermann <stzi@linux.ibm.com> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> >> --- >> monitor.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/monitor.c b/monitor.c >> index 7af1f18..7942f9f 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -1034,9 +1034,12 @@ static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds, >> } else { >> help_cmd_dump_one(mon, cmd, args, arg_index); >> } >> - break; >> + return; >> } >> } >> + >> + /* Entry not found */ >> + monitor_printf(mon, "unknown command: '%s'\n", args[arg_index]); > > Thanks, that does suffer from a similar bug to the one you fixed a > few months back in 317c52cc6aa0d ('monitor: report entirety of hmp > command on error'): > > (qemu) help foo > unknown command: 'foo' > (qemu) help info foo > unknown command: 'foo' Yeah... my thinking was that "info" is a correct command, so let's instead only report to the user just the piece that was incorrect. If it makes better sense to include the whole "info foo" piece, it's certainly doable... whichever makes the most sense. Thoughts? > > Dave > (And yes, please cc me, otherwise I can miss them) > Will do :) >> } >> >> static void help_cmd(Monitor *mon, const char *name) >> -- >> 2.7.4 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
On 07/19/2018 11:39 AM, Collin Walling wrote: > On 07/19/2018 12:31 PM, Markus Armbruster wrote: >> You neglected to cc: maintainers. Cc'ing them increases the odds your >> patch will be noticed and picked up. You can use >> scripts/get_maintainer.pl to find maintainers. You don't have to do >> anything for this patch; it got noticed anyway. >> >> David, this is yours :) > > Very true. Was a minor fix that I thought I'd just toss it out there and > let anyone view it if they had the time. Will be more aware of who to CC > next time around. Or automate it: https://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer suggests: git config sendemail.cccmd 'scripts/get_maintainer.pl --nogit-fallback'
* Collin Walling (walling@linux.ibm.com) wrote: > On 07/19/2018 03:18 PM, Dr. David Alan Gilbert wrote: > > * Collin Walling (walling@linux.ibm.com) wrote: > >> When typing 'help' followed by an unknown command, QEMU will > >> not print anything to the command line to let the user know > >> they typed a bad command. Let's fix this by printing a message > >> to the monitor when this happens. For example: > >> > >> (qemu) help xyz > >> unknown command: 'xyz' > >> > >> Reported-by: Stefan Zimmermann <stzi@linux.ibm.com> > >> Signed-off-by: Collin Walling <walling@linux.ibm.com> > >> --- > >> monitor.c | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/monitor.c b/monitor.c > >> index 7af1f18..7942f9f 100644 > >> --- a/monitor.c > >> +++ b/monitor.c > >> @@ -1034,9 +1034,12 @@ static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds, > >> } else { > >> help_cmd_dump_one(mon, cmd, args, arg_index); > >> } > >> - break; > >> + return; > >> } > >> } > >> + > >> + /* Entry not found */ > >> + monitor_printf(mon, "unknown command: '%s'\n", args[arg_index]); > > > > Thanks, that does suffer from a similar bug to the one you fixed a > > few months back in 317c52cc6aa0d ('monitor: report entirety of hmp > > command on error'): > > > > (qemu) help foo > > unknown command: 'foo' > > (qemu) help info foo > > unknown command: 'foo' > > Yeah... my thinking was that "info" is a correct command, so let's instead only > report to the user just the piece that was incorrect. > > If it makes better sense to include the whole "info foo" piece, it's certainly > doable... whichever makes the most sense. Thoughts? Yes, I'd prefer the full version; unless it makes it particularly complicated. > > > > Dave > > (And yes, please cc me, otherwise I can miss them) > > > > Will do :) Thanks, Dave > >> } > >> > >> static void help_cmd(Monitor *mon, const char *name) > >> -- > >> 2.7.4 > >> > >> > > -- > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > > > > -- > Respectfully, > - Collin Walling > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 07/20/2018 12:40 PM, Eric Blake wrote: > On 07/19/2018 11:39 AM, Collin Walling wrote: >> On 07/19/2018 12:31 PM, Markus Armbruster wrote: >>> You neglected to cc: maintainers. Cc'ing them increases the odds your >>> patch will be noticed and picked up. You can use >>> scripts/get_maintainer.pl to find maintainers. You don't have to do >>> anything for this patch; it got noticed anyway. >>> >>> David, this is yours :) >> >> Very true. Was a minor fix that I thought I'd just toss it out there and >> let anyone view it if they had the time. Will be more aware of who to CC >> next time around. > > Or automate it: https://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer suggests: > > git config sendemail.cccmd 'scripts/get_maintainer.pl --nogit-fallback' > This is very helpful, thank you.
diff --git a/monitor.c b/monitor.c index 7af1f18..7942f9f 100644 --- a/monitor.c +++ b/monitor.c @@ -1034,9 +1034,12 @@ static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds, } else { help_cmd_dump_one(mon, cmd, args, arg_index); } - break; + return; } } + + /* Entry not found */ + monitor_printf(mon, "unknown command: '%s'\n", args[arg_index]); } static void help_cmd(Monitor *mon, const char *name)
When typing 'help' followed by an unknown command, QEMU will not print anything to the command line to let the user know they typed a bad command. Let's fix this by printing a message to the monitor when this happens. For example: (qemu) help xyz unknown command: 'xyz' Reported-by: Stefan Zimmermann <stzi@linux.ibm.com> Signed-off-by: Collin Walling <walling@linux.ibm.com> --- monitor.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)