Eric Blake <ebl...@redhat.com> writes: > On 10/11/20 2:35 AM, Markus Armbruster wrote: >> From: Kevin Wolf <kw...@redhat.com> >> This adds a special meaning for 'help' and '?' as options to the >> keyval >> parser. Instead of being an error (because of a missing value) or a >> value for an implied key, they now request help, which is a new boolean >> output of the parser in addition to the QDict. >> A new parameter 'p_help' is added to keyval_parse() that contains on >> return whether help was requested. If NULL is passed, requesting help >> results in an error and all other cases work like before. >> > >> + >> + /* "help" by itself, without implied key */ >> + qdict = keyval_parse("help", NULL, &help, &error_abort); >> + g_assert_cmpuint(qdict_size(qdict), ==, 0); >> + g_assert(help); >> + qobject_unref(qdict); >> + >> + /* "help" by itself, with implied key */ >> + qdict = keyval_parse("help", "implied", &help, &error_abort); >> + g_assert_cmpuint(qdict_size(qdict), ==, 0); >> + g_assert(help); >> + qobject_unref(qdict); >> + >> + /* "help" when no help is available, without implied key */ >> + qdict = keyval_parse("help", NULL, NULL, &err); >> + error_free_or_abort(&err); >> + g_assert(!qdict); >> + >> + /* "help" when no help is available, with implied key */ >> + qdict = keyval_parse("help", "implied", NULL, &err); >> + error_free_or_abort(&err); >> + g_assert(!qdict); >> + >> + /* Key "help" */ >> + qdict = keyval_parse("help=on", NULL, &help, &error_abort); >> + g_assert_cmpuint(qdict_size(qdict), ==, 1); >> + g_assert_cmpstr(qdict_get_try_str(qdict, "help"), ==, "on"); >> + g_assert(!help); >> + qobject_unref(qdict); >> + >> + /* "help" followed by crap, without implied key */ >> + qdict = keyval_parse("help.abc", NULL, &help, &err); >> + error_free_or_abort(&err); >> + g_assert(!qdict); >> + >> + /* "help" followed by crap, with implied key */ >> + qdict = keyval_parse("help.abc", "implied", &help, &err); >> + g_assert_cmpuint(qdict_size(qdict), ==, 1); >> + g_assert_cmpstr(qdict_get_try_str(qdict, "implied"), ==, "help.abc"); >> + g_assert(!help); >> + qobject_unref(qdict); >> + >> + /* "help" with other stuff, without implied key */ >> + qdict = keyval_parse("number=42,help,foo=bar", NULL, &help, >> &error_abort); >> + g_assert_cmpuint(qdict_size(qdict), ==, 2); >> + g_assert_cmpstr(qdict_get_try_str(qdict, "number"), ==, "42"); >> + g_assert_cmpstr(qdict_get_try_str(qdict, "foo"), ==, "bar"); >> + g_assert(help); >> + qobject_unref(qdict); >> + >> + /* "help" with other stuff, with implied key */ >> + qdict = keyval_parse("val,help,foo=bar", "implied", &help, >> &error_abort); >> + g_assert_cmpuint(qdict_size(qdict), ==, 2); >> + g_assert_cmpstr(qdict_get_try_str(qdict, "implied"), ==, "val"); >> + g_assert_cmpstr(qdict_get_try_str(qdict, "foo"), ==, "bar"); >> + g_assert(help); >> + qobject_unref(qdict); > > Is it worth checking that "helper" with implied key is a value, not help?
Case /* "help" followed by crap, with implied key */ covers this, doesn't it? >> +++ b/util/keyval.c >> @@ -14,10 +14,11 @@ >> * KEY=VALUE,... syntax: >> * >> * key-vals = [ key-val { ',' key-val } [ ',' ] ] >> - * key-val = key '=' val >> + * key-val = key '=' val | help >> * key = key-fragment { '.' key-fragment } >> * key-fragment = / [^=,.]+ / >> * val = { / [^,]+ / | ',,' } >> + * help = 'help | '?' > > Missing ' Kevin fixed it up. > Otherwise > Reviewed-by: Eric Blake <ebl...@redhat.com> Thanks!