[Xfce4-commits] <terminal:master> Split option parsing in 2 stages.

Nick Schermer noreply at xfce.org
Tue Dec 8 10:56:03 CET 2009


Updating branch refs/heads/master
         to fa00dd53c92c763d35bca2a0c94623ed25fe8d9e (commit)
       from f9852ea0d8af12b0e4e61b8fd6a6806a962ac4d6 (commit)

commit fa00dd53c92c763d35bca2a0c94623ed25fe8d9e
Author: Nick Schermer <nick at xfce.org>
Date:   Tue Dec 8 10:42:26 2009 +0100

    Split option parsing in 2 stages.
    
    Previously we check the command line options 2 times during
    startup (well 3 times when invoking a remote instance). Split
    this and only check the basic options (without error checking)
    we need in main(), and parse the other options in a new
    function terminal_window_attr_parse(). Also use the new
    error reply from D-Bus when option parsing in the remote
    instance failed, so we don't try that twice.

 terminal/main.c             |   54 +++++++------
 terminal/terminal-app.c     |    9 ++-
 terminal/terminal-options.c |  188 +++++++++++++++++--------------------------
 terminal/terminal-options.h |   27 +++----
 4 files changed, 121 insertions(+), 157 deletions(-)

diff --git a/terminal/main.c b/terminal/main.c
index 066624c..ff9a330 100644
--- a/terminal/main.c
+++ b/terminal/main.c
@@ -134,7 +134,9 @@ usage (void)
 int
 main (int argc, char **argv)
 {
-  TerminalOptions *options;
+  gboolean         show_help = FALSE;
+  gboolean         show_version = FALSE;
+  gboolean         disable_server = FALSE;
   GdkModifierType  modifiers;
   TerminalApp     *app;
   const gchar     *startup_id;
@@ -160,15 +162,10 @@ main (int argc, char **argv)
   g_log_set_always_fatal (G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_WARNING);
 #endif
 
-  if (!terminal_options_parse (argc, argv, NULL, &options, &error))
-    {
-      g_printerr ("%s\n", error->message);
-      g_error_free (error);
+  /* parse some options we need in main, not the windows attrs */
+  terminal_options_parse (argc, argv, &show_help, &show_version, &disable_server);
 
-      return EXIT_FAILURE;
-    }
-
-  if (G_UNLIKELY (options->show_version))
+  if (G_UNLIKELY (show_version))
     {
       g_print (_("%s (Xfce %s)\n\n"
                  "Copyright (c) %s\n"
@@ -183,7 +180,7 @@ main (int argc, char **argv)
                  PACKAGE_BUGREPORT);
       return EXIT_SUCCESS;
     }
-  else if (G_UNLIKELY (options->show_help))
+  else if (G_UNLIKELY (show_help))
     {
       usage ();
       return EXIT_SUCCESS;
@@ -214,7 +211,7 @@ main (int argc, char **argv)
   nargv[nargc] = NULL;
 
 #ifdef HAVE_DBUS
-  if (!options->disable_server)
+  if (!disable_server)
     {
       /* try to connect to an existing Terminal service */
       if (terminal_dbus_invoke_launch (nargc, nargv, &error))
@@ -223,23 +220,33 @@ main (int argc, char **argv)
         }
       else
         {
-          /* handle "User mismatch" special */
           if (error->domain == TERMINAL_ERROR
               && error->code == TERMINAL_ERROR_USER_MISMATCH)
             {
               /* don't try to establish another service here */
-              options->disable_server = TRUE;
+              disable_server = TRUE;
 #ifndef NDEBUG
               g_debug ("User mismatch when invoking remote terminal: %s", error->message);
+#endif
             }
+          else if (error->domain == TERMINAL_ERROR
+                   && error->code == TERMINAL_ERROR_OPTIONS)
+            {
+              /* options were not parsed succesfully, don't try that again below */
+              g_printerr ("%s\n", error->message);
+              g_error_free (error);
+              g_strfreev (nargv);
+              return EXIT_FAILURE;
+            }
+#ifndef NDEBUG
           else
             {
-              g_debug ("No running instance found: %s", error->message);
-#endif
+              g_debug ("D-Bus reply error: %s (%s: %d)", error->message,
+                       g_quark_to_string (error->domain), error->code);
             }
+#endif
 
-          g_error_free (error);
-          error = NULL;
+          g_clear_error (&error);
         }
     }
 #endif /* !HAVE_DBUS */
@@ -260,27 +267,26 @@ main (int argc, char **argv)
   app = g_object_new (TERMINAL_TYPE_APP, NULL);
 
 #ifdef HAVE_DBUS
-  if (!options->disable_server)
+  if (!disable_server)
     {
       if (!terminal_dbus_register_service (app, &error))
         {
           g_printerr (_("Unable to register terminal service: %s\n"), error->message);
-          g_error_free (error);
-          error = NULL;
+          g_clear_error (&error);
         }
     }
 #endif /* !HAVE_DBUS */
 
   if (!terminal_app_process (app, nargv, nargc, &error))
     {
-      g_printerr (_("Unable to launch terminal: %s\n"), error->message);
+      /* parsing one of the arguments failed */
+      g_printerr ("%s\n", error->message);
       g_error_free (error);
+      g_object_unref (G_OBJECT (app));
+      g_strfreev (nargv);
       return EXIT_FAILURE;
     }
 
-  /* free parsed options */
-  terminal_options_free (options);
-
   /* free temporary arguments */
   g_strfreev (nargv);
 
diff --git a/terminal/terminal-app.c b/terminal/terminal-app.c
index 86cf506..3401ed4 100644
--- a/terminal/terminal-app.c
+++ b/terminal/terminal-app.c
@@ -457,13 +457,16 @@ terminal_app_process (TerminalApp  *app,
 {
   GSList *attrs, *lp;
 
-  if (!terminal_options_parse (argc, argv, &attrs, NULL, error))
+  attrs = terminal_window_attr_parse (argc, argv, error);
+  if (G_UNLIKELY (attrs == NULL))
     return FALSE;
 
   for (lp = attrs; lp != NULL; lp = lp->next)
-    terminal_app_open_window (app, lp->data);
+    {
+      terminal_app_open_window (app, lp->data);
+      terminal_window_attr_free (lp->data);
+    }
 
-  g_slist_foreach (attrs, (GFunc) terminal_window_attr_free, NULL);
   g_slist_free (attrs);
 
   return TRUE;
diff --git a/terminal/terminal-options.c b/terminal/terminal-options.c
index a22dc27..ecc1b27 100644
--- a/terminal/terminal-options.c
+++ b/terminal/terminal-options.c
@@ -139,41 +139,58 @@ terminal_tab_attr_free (TerminalTabAttr *attr)
 
 
 
+void
+terminal_options_parse (gint       argc,
+                        gchar    **argv,
+                        gboolean  *show_help,
+                        gboolean  *show_version,
+                        gboolean  *disable_server)
+{
+  gint n;
+
+  for (n = 1; n < argc; ++n)
+    {
+      /* all arguments should atleast start with a dash */
+      if (argv[n] == NULL || *argv[n] != '-')
+        continue;
+
+      if (terminal_option_cmp ("help", 'h', argc, argv, &n, NULL))
+        *show_help = TRUE;
+      else if (terminal_option_cmp ("version", 'V', argc, argv, &n, NULL))
+        *show_version = TRUE;
+      else if (terminal_option_cmp ("disable-server", 0, argc, argv, &n, NULL))
+        *disable_server = TRUE;
+    }
+}
+
+
+
 /**
- * terminal_options_parse:
+ * terminal_window_attr_parse:
  * @argc            :
  * @argv            :
- * @attrs_return    :
- * @options_return  :
  * @error           :
  *
- * Return value: %TRUE on success.
+ * Return value: %NULL on failure.
  **/
-gboolean
-terminal_options_parse (gint              argc,
-                        gchar           **argv,
-                        GSList          **attrs_return,
-                        TerminalOptions **options_return,
-                        GError          **error)
+GSList *
+terminal_window_attr_parse (gint              argc,
+                            gchar           **argv,
+                            GError          **error)
 {
-  TerminalWindowAttr *win_attr = NULL;
-  TerminalTabAttr    *tab_attr = NULL;
+  TerminalWindowAttr *win_attr;
+  TerminalTabAttr    *tab_attr;
   gchar              *default_directory = NULL;
   gchar              *default_display = NULL;
   gchar              *s;
   GSList             *tp, *wp;
+  GSList             *attrs;
   gint                n;
   TerminalVisibility  visible;
 
-  if (attrs_return != NULL)
-    {
-      win_attr = terminal_window_attr_new ();
-      tab_attr = win_attr->tabs->data;
-      *attrs_return = g_slist_prepend (NULL, win_attr);
-    }
-
-  if (options_return != NULL)
-    *options_return = g_slice_new0 (TerminalOptions);
+  win_attr = terminal_window_attr_new ();
+  tab_attr = win_attr->tabs->data;
+  attrs = g_slist_prepend (NULL, win_attr);
 
   for (n = 1; n < argc; ++n)
     {
@@ -181,6 +198,7 @@ terminal_options_parse (gint              argc,
       if (argv[n] == NULL || *argv[n] != '-')
         goto unknown_option;
 
+
       if (terminal_option_cmp ("default-display", 0, argc, argv, &n, &s))
         {
           if (G_UNLIKELY (s == NULL))
@@ -212,36 +230,6 @@ terminal_options_parse (gint              argc,
               default_directory = g_strdup (s);
             }
         }
-      else if (terminal_option_cmp ("help", 'h', argc, argv, &n, NULL))
-        {
-          if (options_return != NULL)
-            (*options_return)->show_help = TRUE;
-        }
-      else if (terminal_option_cmp ("version", 'V', argc, argv, &n, NULL))
-        {
-          if (options_return != NULL)
-            (*options_return)->show_version = TRUE;
-        }
-      else if (terminal_option_cmp ("disable-server", 0, argc, argv, &n, NULL))
-        {
-          if (options_return != NULL)
-            (*options_return)->disable_server = TRUE;
-        }
-      else if (terminal_option_cmp ("sm-client-id", 0, argc, argv, &n, &s))
-        {
-          if (G_UNLIKELY (s == NULL))
-            {
-              g_set_error (error, G_SHELL_ERROR, G_SHELL_ERROR_FAILED,
-                           _("Option \"--sm-client-id\" requires specifying "
-                             "the session id as its parameter"));
-              goto failed;
-            }
-          else if (options_return != NULL)
-            {
-              g_free ((*options_return)->session_id);
-              (*options_return)->session_id = g_strdup (s);
-            }
-        }
       else if (terminal_option_cmp ("execute", 'x', argc, argv, &n, NULL))
         {
           if (++n >= argc)
@@ -251,7 +239,7 @@ terminal_options_parse (gint              argc,
                              "to run on the rest of the command line"));
               goto failed;
             }
-          else if (tab_attr != NULL)
+          else
             {
               g_strfreev (tab_attr->command);
               tab_attr->command = g_strdupv (argv + n);
@@ -268,7 +256,7 @@ terminal_options_parse (gint              argc,
                              "the command to run as its parameter"));
               goto failed;
             }
-          else if (tab_attr != NULL)
+          else
             {
               g_strfreev (tab_attr->command);
               tab_attr->command = NULL;
@@ -285,7 +273,7 @@ terminal_options_parse (gint              argc,
                              "the working directory as its parameter"));
               goto failed;
             }
-          else if (tab_attr != NULL)
+          else
             {
               g_free (tab_attr->directory);
               tab_attr->directory = g_strdup (s);
@@ -300,7 +288,7 @@ terminal_options_parse (gint              argc,
                              "the title as its parameter"));
               goto failed;
             }
-          else if (tab_attr != NULL)
+          else
             {
               g_free (tab_attr->title);
               tab_attr->title = g_strdup (s);
@@ -308,8 +296,7 @@ terminal_options_parse (gint              argc,
         }
       else if (terminal_option_cmp ("hold", 'H', argc, argv, &n, NULL))
         {
-          if (tab_attr != NULL)
-            tab_attr->hold = TRUE;
+          tab_attr->hold = TRUE;
         }
       else if (terminal_option_cmp ("display", 0, argc, argv, &n, &s))
         {
@@ -320,7 +307,7 @@ terminal_options_parse (gint              argc,
                              "the X display as its parameters"));
               goto failed;
             }
-          else if (win_attr != NULL)
+          else
             {
               g_free (win_attr->display);
               win_attr->display = g_strdup (s);
@@ -335,7 +322,7 @@ terminal_options_parse (gint              argc,
                              "the window geometry as its parameter"));
               goto failed;
             }
-          else if (win_attr != NULL)
+          else
             {
               g_free (win_attr->geometry);
               win_attr->geometry = g_strdup (s);
@@ -350,7 +337,7 @@ terminal_options_parse (gint              argc,
                              "the window role as its parameter"));
               goto failed;
             }
-          else if (win_attr != NULL)
+          else
             {
               g_free (win_attr->role);
               win_attr->role = g_strdup (s);
@@ -365,7 +352,7 @@ terminal_options_parse (gint              argc,
                              "the startup id as its parameter"));
               goto failed;
             }
-          else if (win_attr != NULL)
+          else
             {
               g_free (win_attr->startup_id);
               win_attr->startup_id = g_strdup (s);
@@ -380,7 +367,7 @@ terminal_options_parse (gint              argc,
                              "an icon name or filename as its parameter"));
               goto failed;
             }
-          else if (win_attr != NULL)
+          else
             {
               g_free (win_attr->icon);
               win_attr->icon = g_strdup (s);
@@ -388,45 +375,41 @@ terminal_options_parse (gint              argc,
         }
       else if (terminal_option_show_hide_cmp ("menubar", argc, argv, &n, &visible))
         {
-          if (win_attr != NULL)
-            win_attr->menubar = visible;
+          win_attr->menubar = visible;
         }
       else if (terminal_option_cmp ("fullscreen", 0, argc, argv, &n, NULL))
         {
-          if (win_attr != NULL)
-            win_attr->fullscreen = TRUE;
+          win_attr->fullscreen = TRUE;
         }
       else if (terminal_option_cmp ("maximize", 0, argc, argv, &n, NULL))
         {
-          if (win_attr != NULL)
-            win_attr->maximize = TRUE;
+          win_attr->maximize = TRUE;
         }
       else if (terminal_option_show_hide_cmp ("borders", argc, argv, &n, &visible))
         {
-          if (win_attr != NULL)
-            win_attr->borders = visible;
+          win_attr->borders = visible;
         }
       else if (terminal_option_show_hide_cmp ("toolbars", argc, argv, &n, &visible))
         {
-          if (win_attr != NULL)
-            win_attr->toolbars = visible;
+          win_attr->toolbars = visible;
         }
       else if (terminal_option_cmp ("tab", 0, argc, argv, &n, NULL))
         {
-          if (win_attr != NULL)
-            {
-              tab_attr = g_slice_new0 (TerminalTabAttr);
-              win_attr->tabs = g_slist_append (win_attr->tabs, tab_attr);
-            }
+          tab_attr = g_slice_new0 (TerminalTabAttr);
+          win_attr->tabs = g_slist_append (win_attr->tabs, tab_attr);
         }
       else if (terminal_option_cmp ("window", 0, argc, argv, &n, NULL))
         {
-          if (attrs_return != NULL)
-            {
-              win_attr = terminal_window_attr_new ();
-              tab_attr = win_attr->tabs->data;
-              *attrs_return = g_slist_append (*attrs_return, win_attr);
-            }
+          win_attr = terminal_window_attr_new ();
+          tab_attr = win_attr->tabs->data;
+          attrs = g_slist_append (attrs, win_attr);
+        }
+      else if (terminal_option_cmp ("help", 'h', argc, argv, &n, NULL)
+               || terminal_option_cmp ("version", 'V', argc, argv, &n, NULL)
+               || terminal_option_cmp ("disable-server", 0, argc, argv, &n, NULL)
+               || terminal_option_cmp ("sm-client-id", 0, argc, argv, &n, NULL))
+        {
+          /* options we can ignore */
         }
       else
         {
@@ -438,10 +421,9 @@ unknown_option:
     }
 
   /* substitute default working directory and default display if any */
-  if (attrs_return != NULL
-      && (default_display != NULL || default_directory != NULL))
+  if (default_display != NULL || default_directory != NULL)
     {
-      for (wp = *attrs_return; wp != NULL; wp = wp->next)
+      for (wp = attrs; wp != NULL; wp = wp->next)
         {
           win_attr = wp->data;
           for (tp = win_attr->tabs; tp != NULL; tp = tp->next)
@@ -458,38 +440,18 @@ unknown_option:
   g_free (default_directory);
   g_free (default_display);
 
-  return TRUE;
+  return attrs;
 
 failed:
-  if (options_return != NULL)
-    {
-      terminal_options_free (*options_return);
-      *options_return = NULL;
-    }
-  if (attrs_return != NULL)
-    {
-      for (wp = *attrs_return; wp != NULL; wp = wp->next)
-        terminal_window_attr_free (wp->data);
-      g_slist_free (*attrs_return);
-    }
-  g_free (default_directory);
-  g_free (default_display);
-
-  return FALSE;
-}
 
+  for (wp = attrs; wp != NULL; wp = wp->next)
+    terminal_window_attr_free (wp->data);
+  g_slist_free (attrs);
 
+  g_free (default_directory);
+  g_free (default_display);
 
-/**
- **/
-void
-terminal_options_free (TerminalOptions *options)
-{
-  if (G_LIKELY (options != NULL))
-    {
-      g_free (options->session_id);
-      g_slice_free (TerminalOptions, options);
-    }
+  return NULL;
 }
 
 
diff --git a/terminal/terminal-options.h b/terminal/terminal-options.h
index 72f3f29..af0a855 100644
--- a/terminal/terminal-options.h
+++ b/terminal/terminal-options.h
@@ -26,7 +26,6 @@
 
 G_BEGIN_DECLS
 
-typedef struct _TerminalOptions    TerminalOptions;
 typedef struct _TerminalTabAttr    TerminalTabAttr;
 typedef struct _TerminalWindowAttr TerminalWindowAttr;
 typedef enum   _TerminalVisibility TerminalVisibility;
@@ -38,14 +37,6 @@ enum _TerminalVisibility
   TERMINAL_VISIBILITY_HIDE
 };
 
-struct _TerminalOptions
-{
-  gchar    *session_id;
-  gboolean  show_help;
-  gboolean  show_version;
-  gboolean  disable_server;
-};
-
 struct _TerminalTabAttr
 {
   gchar    **command;
@@ -69,17 +60,19 @@ struct _TerminalWindowAttr
   gboolean             maximize;
 };
 
-gboolean           terminal_options_parse    (gint                 argc,
-                                              gchar              **argv,
-                                              GSList             **attrs_return,
-                                              TerminalOptions    **options_return,
-                                              GError             **error);
+void                terminal_options_parse     (gint                 argc,
+                                                gchar              **argv,
+                                                gboolean            *show_help,
+                                                gboolean            *show_version,
+                                                gboolean            *disable_server);
 
-void                terminal_options_free     (TerminalOptions     *options);
+GSList             *terminal_window_attr_parse (gint                 argc,
+                                                gchar              **argv,
+                                                GError             **error);
 
-TerminalWindowAttr *terminal_window_attr_new  (void);
+TerminalWindowAttr *terminal_window_attr_new   (void);
 
-void                terminal_window_attr_free (TerminalWindowAttr  *attr);
+void                terminal_window_attr_free  (TerminalWindowAttr  *attr);
 
 G_END_DECLS
 



More information about the Xfce4-commits mailing list