From 8cddb54659582042eaede0da158c3ab40105bada Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Mon, 1 Apr 2013 15:01:20 -0400 Subject: [PATCH] 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. https://bugzilla.gnome.org/show_bug.cgi?id=688954 --- docs/reference/gio/gio-sections.txt | 3 ++ gio/gaction.c | 100 ++++++++++++++++++++++++++++++++++++ gio/gaction.h | 6 +++ gio/gmenu.c | 33 +++++------- gio/tests/actions.c | 75 +++++++++++++++++++++++++++ 5 files changed, 196 insertions(+), 21 deletions(-) diff --git a/docs/reference/gio/gio-sections.txt b/docs/reference/gio/gio-sections.txt index 03e8400..add972f 100644 --- a/docs/reference/gio/gio-sections.txt +++ b/docs/reference/gio/gio-sections.txt @@ -3093,6 +3093,9 @@ g_action_get_state g_action_change_state g_action_activate + +g_action_parse_detailed_name + g_action_get_type G_TYPE_ACTION diff --git a/gio/gaction.c b/gio/gaction.c index 3329830..e6b42da 100644 --- a/gio/gaction.c +++ b/gio/gaction.c @@ -23,6 +23,8 @@ #include "gaction.h" #include "glibintl.h" +#include + G_DEFINE_INTERFACE (GAction, g_action, G_TYPE_OBJECT) /** @@ -389,3 +391,101 @@ g_action_activate (GAction *action, if (parameter != NULL) g_variant_unref (parameter); } + +/** + * g_action_parse_detailed_name: + * @detailed_name: a detailed action name + * @action_name: (out): the action name + * @target_value: (out): the target value, or %NULL for no target + * @error: a pointer to a %NULL #GError, or %NULL + * + * Parses a detailed action name into its separate name and target + * components. + * + * Detailed action names can have three formats. + * + * The first format is used to represent an action name with no target + * value and consists of just an action name containing no whitespace + * nor the characters ':', '(' or ')'. For example: "app.action". + * + * The second format is used to represent an action with a string-typed + * target value. The action name and target value are separated by a + * double colon ("::"). For example: "app.action::target". + * + * The third format is used to represent an action with an + * arbitrarily-typed target value. The target value follows the action + * name, surrounded in parens. For example: "app.action(42)". The + * target value is parsed using g_variant_parse(). If a tuple-typed + * value is desired, it must be specified in the same way, resulting in + * two sets of parens, for example: "app.action((1,2,3))". + * + * Returns: %TRUE if successful, else %FALSE with @error set + * + * Since: 2.38 + **/ +gboolean +g_action_parse_detailed_name (const gchar *detailed_name, + gchar **action_name, + GVariant **target_value, + GError **error) +{ + const gchar *target; + gsize target_len; + gsize base_len; + + /* We decide which format we have based on which we see first between + * '::' '(' and '\0'. + */ + + if (*detailed_name == '\0' || *detailed_name == ' ') + goto bad_fmt; + + base_len = strcspn (detailed_name, ": ()"); + target = detailed_name + base_len; + target_len = strlen (target); + + switch (target[0]) + { + case ' ': + case ')': + goto bad_fmt; + + case ':': + if (target[1] != ':') + goto bad_fmt; + + *target_value = g_variant_ref_sink (g_variant_new_string (target + 2)); + break; + + case '(': + { + if (target[target_len - 1] != ')') + goto bad_fmt; + + *target_value = g_variant_parse (NULL, target + 1, target + target_len - 1, NULL, error); + if (*target_value == NULL) + goto bad_fmt; + } + break; + + case '\0': + *target_value = NULL; + break; + } + + *action_name = g_strndup (detailed_name, base_len); + + return TRUE; + +bad_fmt: + if (error) + { + if (*error == NULL) + g_set_error (error, G_VARIANT_PARSE_ERROR, G_VARIANT_PARSE_ERROR_FAILED, + "Detailed action name '%s' has invalid format", detailed_name); + else + g_prefix_error (error, "Detailed action name '%s' has invalid format: ", detailed_name); + } + + return FALSE; +} diff --git a/gio/gaction.h b/gio/gaction.h index 5f846ca..b153db6 100644 --- a/gio/gaction.h +++ b/gio/gaction.h @@ -81,6 +81,12 @@ void g_action_change_state (GAction GLIB_AVAILABLE_IN_ALL void g_action_activate (GAction *action, GVariant *parameter); + +GLIB_AVAILABLE_IN_2_38 +gboolean g_action_parse_detailed_name (const gchar *detailed_name, + gchar **action_name, + GVariant **target_value, + GError **error); G_END_DECLS #endif /* __G_ACTION_H__ */ diff --git a/gio/gmenu.c b/gio/gmenu.c index d0b4130..1fa132f1 100644 --- a/gio/gmenu.c +++ b/gio/gmenu.c @@ -23,6 +23,7 @@ #include "gmenu.h" +#include "gaction.h" #include /** @@ -1056,14 +1057,8 @@ g_menu_item_set_action_and_target (GMenuItem *menu_item, * * Sets the "action" and possibly the "target" attribute of @menu_item. * - * If @detailed_action contains a double colon ("::") then it is used as - * a separator between an action name and a target string. In this - * case, this call is equivalent to calling - * g_menu_item_set_action_and_target() with the part before the "::" and - * with a string-type #GVariant containing the part following the "::". - * - * If @detailed_action doesn't contain "::" then the action is set to - * the given string (verbatim) and the target value is unset. + * The format of @detailed_action is the same format parsed by + * g_action_parse_detailed_name(). * * See g_menu_item_set_action_and_target() or * g_menu_item_set_action_and_target_value() for more flexible (but @@ -1078,21 +1073,17 @@ void g_menu_item_set_detailed_action (GMenuItem *menu_item, const gchar *detailed_action) { - const gchar *sep; - - sep = strstr (detailed_action, "::"); - - if (sep != NULL) - { - gchar *action; + GError *error = NULL; + GVariant *target; + gchar *name; - action = g_strndup (detailed_action, sep - detailed_action); - g_menu_item_set_action_and_target (menu_item, action, "s", sep + 2); - g_free (action); - } + if (!g_action_parse_detailed_name (detailed_action, &name, &target, &error)) + g_error ("g_menu_item_set_detailed_action: %s", error->message); - else - g_menu_item_set_action_and_target_value (menu_item, detailed_action, NULL); + g_menu_item_set_action_and_target_value (menu_item, name, target); + if (target) + g_variant_unref (target); + g_free (name); } /** diff --git a/gio/tests/actions.c b/gio/tests/actions.c index 74bcd9a..b9ba675 100644 --- a/gio/tests/actions.c +++ b/gio/tests/actions.c @@ -1,5 +1,6 @@ #include #include +#include #include "gdbus-sessionbus.h" @@ -387,6 +388,79 @@ test_entries (void) g_object_unref (actions); } +static void +test_parse_detailed (void) +{ + struct { + const gchar *detailed; + const gchar *expected_name; + const gchar *expected_target; + const gchar *expected_error; + } testcases[] = { + { "abc", "abc", NULL, NULL }, + { " abc", NULL, NULL, "invalid format" }, + { " abc", NULL, NULL, "invalid format" }, + { "abc:", NULL, NULL, "invalid format" }, + { ":abc", NULL, NULL, "invalid format" }, + { "abc(", NULL, NULL, "invalid format" }, + { "abc)", NULL, NULL, "invalid format" }, + { "(abc", NULL, NULL, "invalid format" }, + { ")abc", NULL, NULL, "invalid format" }, + { "abc::xyz", "abc", "'xyz'", NULL }, + { "abc('xyz')", "abc", "'xyz'", NULL }, + { "abc(42)", "abc", "42", NULL }, + { "abc(int32 42)", "abc", "42", NULL }, + { "abc(@i 42)", "abc", "42", NULL }, + { "abc (42)", NULL, NULL, "invalid format" }, + { "abc(42abc)", NULL, NULL, "invalid character in number" }, + { "abc(42, 4)", "abc", "(42, 4)", "expected end of input" }, + { "abc(42,)", "abc", "(42,)", "expected end of input" } + }; + gint i; + + for (i = 0; i < G_N_ELEMENTS (testcases); i++) + { + GError *error = NULL; + GVariant *target; + gboolean success; + gchar *name; + + success = g_action_parse_detailed_name (testcases[i].detailed, &name, &target, &error); + g_assert (success == (error == NULL)); + if (success && testcases[i].expected_error) + g_error ("Unexpected success on '%s'. Expected error containing '%s'", + testcases[i].detailed, testcases[i].expected_error); + + if (!success && !testcases[i].expected_error) + g_error ("Unexpected failure on '%s': %s", testcases[i].detailed, error->message); + + if (!success) + { + if (!strstr (error->message, testcases[i].expected_error)) + g_error ("Failure message '%s' for string '%s' did not contained expected substring '%s'", + error->message, testcases[i].detailed, testcases[i].expected_error); + + g_error_free (error); + continue; + } + + g_assert_cmpstr (name, ==, testcases[i].expected_name); + g_assert ((target == NULL) == (testcases[i].expected_target == NULL)); + if (target) + { + GVariant *expected; + + expected = g_variant_parse (NULL, testcases[i].expected_target, NULL, NULL, NULL); + g_assert (expected); + + g_assert (g_variant_equal (expected, target)); + g_variant_unref (expected); + g_variant_unref (target); + } + + g_free (name); + } +} GHashTable *activation_counts; @@ -839,6 +913,7 @@ main (int argc, char **argv) g_test_add_func ("/actions/simplegroup", test_simple_group); g_test_add_func ("/actions/stateful", test_stateful); g_test_add_func ("/actions/entries", test_entries); + g_test_add_func ("/actions/parse-detailed", test_parse_detailed); g_test_add_func ("/actions/dbus/export", test_dbus_export); g_test_add_func ("/actions/dbus/threaded", test_dbus_threaded); g_test_add_func ("/actions/dbus/bug679509", test_bug679509); -- 2.7.4