[glib] gaction: add parser for detailed action names

[Ryan Lortie <ryanl src gnome org>]

commit 8cddb54659582042eaede0da158c3ab40105bada
Author: Ryan Lortie <desrt desrt ca>
Date:   Mon Apr 1 15:01:20 2013 -0400

    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
 
+<SUBSECTION>
+g_action_parse_detailed_name
+
 <SUBSECTION Standard>
 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 <string.h>
+
 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 <string.h>
 
 /**
@@ -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 <gio/gio.h>
 #include <stdlib.h>
+#include <string.h>
 
 #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);