wenchao xia <xiaxia34...@163.com> writes:
> δΊ 2013-1-10 11:30, Wenchao Xia ει:
>>>> -static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
>>>> -{
>>>> - return search_dispatch_table(qmp_cmds, cmdname);
>>>> -}
>>>
>>> Touching qmp_find_cmd() is unrelated to this series.
>>>
>> OK.
>>
>>>> -
>>>> -static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>>> - const char *cmdline,
>>>> - QDict *qdict)
>>>> +/*
>>>> + * Try find the target mon_cmd_t instance, if it have sub_table and
>>>> have string
>>>> + * for it, this function will try search it with remain string, and
>>>> if not
>>>> + * found it return NULL.
>>>> + */
>>>> +static const mon_cmd_t *hmp_parse_command(Monitor *mon,
>>>> + const char *cmdline,
>>>> + mon_cmd_t *table,
>>>> + QDict *qdict)
>>>
>>> Renaming is also not necessary, but if you do it please do in a different
>>> patch.
>>>
>> OK.
>>
>>>> {
>>>> + }
>>>> + return hmp_parse_command(mon, p, cmd->sub_table, qdict);
>>>
>>> This works, and seems less hack then the previous attempt.
>>>
>>> Except for a small detail. This:
>>>
>>> (qemu) info bla
>>>
>>> Currently returns a list of info commands, but with this series it
>>> returns
>>> an error.
>>>
>> Yep, I think command completion is another topic, but I am pretty
>> sure it can be solved in this framework, for that it is not hack and
>> what need to do is, let command completion function knows there may be
>> another sub layer too.
>>
> Sorry I missed your point. Returning an error instead of info
> commands, is made on purpose. IMHO always print the commands
> make user confused, message tipping what is wrong is better.
I agree with the change, but it needs to be mentioned in the commit
message. Separate patch would be even better.
>>> I'd like to get a reviewed-by from Markus before applying this as,
>>> besides
>>> this being his idea, I honestly have a very hard time knowing how to
>>> move old
>>> hmp hackery forward.
I'll review v4.
