GNOME Bugzilla – Bug 688954
extend 'detailed action' syntax, provide parser
Last modified: 2013-04-01 20:57:26 UTC
We currently support the specification of detailed actions like 'foo::bar' to mean action "foo" with the string "bar" as a target. This syntax only works for string-valued targets. I want to add a new syntax that would allow for non-string targets such that foo::bar is short for foo('bar') allowing also to consider the possibility of foo(5) or foo(true). An interesting additional possibility is presented by the fact that given a valid value 'x' in GVariant text format, '(x)' is never valid (since you would have to write '(x,)' to get the one-tuple). That means that we could have foo('bar') as passing argument «'bar'» (string) but then foo('bar',) as passing argument «('bar,')» (1-tuple) and foo('bar', 'baz') as passing argument «('bar', 'baz')» (pair) In any case, this expansion in syntax should probably work consistently everywhere (like in GtkActionable as well, for example) and therefore we should introduce API for parsing one of these strings into a name/target pair: gboolean g_action_parse_detailed_name (const gchar *detailed_name, gchar **action_name, /* out */ GVariant **target, /* out */ GError **error);
Created attachment 229825 [details] [review] gaction: add parser for detailed action names Expand and formalise the syntax for detailed action names, adding a well-documented (and tested) public parser API for them. Port the only GLib-based user of detailed action names to the new API: g_menu_item_set_detailed_action(). The users in Gtk+ will also be ported soon.
Review of attachment 229825 [details] [review]: ::: gio/gaction.c @@ +421,3 @@ + * + * Returns: %TRUE if successful, else %FALSE with @error set + * I think this introduces several restrictions on action names that are nowhere spelled out in our documentation, much less enforced in the code. That should be fixed...in a separate patch. @@ +478,3 @@ + * target, so let's give the error for that case... + * + * First, try without the brackets, recording the error... Couldn't you avoid this altogether by just requiring an outer set of parens, always ? You would then have to write foo((1,2,3)) if you mean a tuple. But really, is that so bad ?
Created attachment 240326 [details] [review] gaction: add parser for detailed action names Expand and formalise the syntax for detailed action names, adding a well-documented (and tested) public parser API for them. Port the only GLib-based user of detailed action names to the new API: g_menu_item_set_detailed_action(). The users in Gtk+ will also be ported soon. As suggested, I simplified the format to remove the special handling of tuples.
Review of attachment 240326 [details] [review]: Looks good ::: gio/tests/actions.c @@ +414,3 @@ + { "abc (42)", NULL, NULL, "invalid format" }, + { "abc(42abc)", NULL, NULL, "invalid character in number" }, + { "abc(42, 4)", "abc", "(42, 4)", NULL }, This case is failing @@ +415,3 @@ + { "abc(42abc)", NULL, NULL, "invalid character in number" }, + { "abc(42, 4)", "abc", "(42, 4)", NULL }, + { "abc(42,)", "abc", "(42,)", NULL } This case should also be failing
Attachment 240326 [details] pushed as 8cddb54 - gaction: add parser for detailed action names Thanks! Updated and pushed.