[patch] for preferences



Hi Gimp developers,

   The entire preferences mechanism is pretty badly broken. This patch
fixes a number of problems, and also gives better error messages if
the gtkrc file is missing or has other problems (e.g. permissions).

   I also found some problems in gtk+'s selection handling, all of
which were problems in the gtk_list_remove_link idiom - it's "A =
g_l_r_l (A, B);", not "g_l_r_l (A, B)". This may not have fixed all of
the problems, but it's getting late and I wanted to send this off.

   There are still some open issues (see the comment in commands.c).

   The first two patches are to gimp (gimprc.c and commands.c). The
third patch is to gtk+ (gtkselection.c). I'm cc'ing gtk-list because
the third patch would seem to be relevant for gtk developers as well.

Raph

Index: app/gimprc.c
===================================================================
RCS file: /home/cvs/gimp/app/gimprc.c,v
retrieving revision 1.6
diff -u -r1.6 gimprc.c
--- gimprc.c	1997/10/25 07:02:22	1.6
+++ gimprc.c	1997/11/04 07:28:39
@@ -153,7 +153,7 @@
 static void gimprc_set_token (char *token, char *value);
 static Argument * gimprc_query (Argument *args);
 static void add_gimp_directory_token (char *gimp_dir);
-static int open_backup_file (char *filename, FILE **fp_new, FILE **fp_old);
+static char* open_backup_file (char *filename, FILE **fp_new, FILE **fp_old);
 
 static ParseInfo parse_info;
 
@@ -365,6 +365,7 @@
   char *cur_line;
   char *prev_line;
   char *str;
+  char *error_msg;
 
   g_assert(updated_options != NULL);
   g_assert(conflicting_options != NULL);
@@ -372,10 +373,11 @@
   gimp_dir = gimp_directory ();
   sprintf (name, "%s/gimprc", gimp_dir);
 
-  if (! open_backup_file (name, &fp_new, &fp_old))
+  error_msg = open_backup_file (name, &fp_new, &fp_old);
+  if (error_msg != NULL)
     {
       /* FIXME: show the error in a nice dialog box */
-      g_warning ("cannot save .gimprc");
+      g_warning (error_msg);
       return;
     }
 
@@ -1543,14 +1545,18 @@
 string_to_str (gpointer val1p,
 	       gpointer val2p)
 {
-  return g_strdup (*((char **)val1p));
+  char *str;
+
+  str = g_malloc (strlen (*((char **)val1p)) + 3);
+  sprintf (str, "%c%s%c", '"', *((char **)val1p), '"');
+  return str;
 }
 
 static char *
 path_to_str (gpointer val1p,
 	     gpointer val2p)
 {
-  return g_strdup (*((char **)val1p));
+  return string_to_str (val1p, val2p);
 }
 
 static char *
@@ -1697,10 +1703,22 @@
   unknown_tokens = g_list_append (unknown_tokens, ut);
 }
 
-static int
+/* Try to:
+
+   1. Open gimprc for reading.
+
+   2. Rename gimprc to gimprc.old.
+
+   3. Open gimprc for writing.
+
+   On success, return NULL. On failure, return a string in English
+   explaining the problem.
+
+   */
+static char *
 open_backup_file (char *filename,
-		  FILE **fp_new,
-		  FILE **fp_old)
+                  FILE **fp_new,
+                  FILE **fp_old)
 {
   char *oldfilename;
 
@@ -1708,25 +1726,37 @@
     Rename the file to *.old, open it for reading and create the new file.
   */
   if ((*fp_old = fopen (filename, "rt")) == NULL)
-    return FALSE;
+    {
+      if (errno == EACCES)
+        return "Can't open gimprc; permission problems";
+      if (errno == ENOENT)
+        return "Can't open gimprc; file does not exist";
+      return "Can't open gimprc, reason unknown";
+    }
 
   oldfilename = g_malloc (strlen (filename) + 5);
   sprintf (oldfilename, "%s.old", filename);
   if (rename (filename, oldfilename) < 0)
     {
       g_free (oldfilename);
-      return FALSE;
+      if (errno == EACCES)
+        return "Can't rename gimprc to gimprc.old; permission problems";
+      if (errno == EISDIR)
+        return "Can't rename gimprc to gimprc.old; gimprc.old is a directory";
+      return "Can't rename gimprc to gimprc.old, reason unknown";
     }
 
   if ((*fp_new = fopen (filename, "wt")) == NULL)
     {
       (void) rename (oldfilename, filename);
       g_free (oldfilename);
-      return FALSE;
+      if (errno == EACCES)
+        return "Can't write to gimprc; permission problems";
+      return "Can't write to gimprc, reason unknown";
     }
 
   g_free (oldfilename);
-  return TRUE;
+  return NULL;
 }
 
 static char*

Index: app/commands.c
===================================================================
RCS file: /home/cvs/gimp/app/commands.c,v
retrieving revision 1.6
diff -u -r1.6 commands.c
--- commands.c	1997/10/25 07:02:19	1.6
+++ commands.c	1997/11/04 07:49:55
@@ -132,6 +132,19 @@
 static   char *       old_plug_in_path;
 static   char *       old_gradient_path;
 
+static   char *       edit_temp_path = NULL;
+static   char *       edit_swap_path = NULL;
+static   char *       edit_brush_path = NULL;
+static   char *       edit_pattern_path = NULL;
+static   char *       edit_palette_path = NULL;
+static   char *       edit_plug_in_path = NULL;
+static   char *       edit_gradient_path = NULL;
+static   int          edit_stingy_memory_use;
+static   int          edit_tile_cache_size;
+static   int          edit_install_cmap;
+static   int          edit_cycled_marching_ants;
+
+
 /*  local functions  */
 static void   image_resize_callback (GtkWidget *, gpointer);
 static void   image_scale_callback (GtkWidget *, gpointer);
@@ -466,6 +479,80 @@
 }
 
 
+/* Some information regarding preferences, compiled by Raph Levien 11/3/97.
+
+   The following preference items cannot be set on the fly (at least
+   according to the existing pref code - it may be that changing them
+   so they're set on the fly is not hard).
+
+   temp-path
+   swap-path
+   brush-path
+   pattern-path
+   plug-in-path
+   palette-path
+   gradient-path
+   stingy-memory-use
+   tile-cache-size
+   install-cmap
+   cycled-marching-ants
+
+   All of these now have variables of the form edit_temp_path, which
+   are copied from the actual variables (e.g. temp_path) the first time
+   the dialog box is started.
+
+   Variables of the form old_temp_path represent the values at the
+   time the dialog is opened - a cancel copies them back from old to
+   the real variables or the edit variables, depending on whether they
+   can be set on the fly.
+
+   Here are the remaining issues as I see them:
+
+   Still no settings for default-brush, default-gradient,
+   default-palette, default-pattern, gamma-correction, color-cube,
+   marching-ants-speed, show-rulers, ruler-units. No widget for
+   confirm-on-close although a lot of stuff is there.
+
+   No UI feedback for the fact that some settings won't take effect
+   until the next Gimp restart.
+
+   The semantics of "save" are a little funny - it only saves the
+   settings that are different from the way they were when the dialog
+   was opened. So you can set something, close the window, open it
+   up again, click "save" and have nothing happen. To change this
+   to more intuitive semantics, we should have a whole set of init_
+   variables that are set the first time the dialog is opened (along
+   with the edit_ variables that are currently set). Then, the save
+   callback checks against the init_ variable rather than the old_.
+
+   */
+
+/* Copy the string from source to destination, freeing the string stored
+   in the destination if there is one there already. */
+static void
+file_prefs_strset (char **dst, char *src)
+{
+  if (*dst != NULL)
+    g_free (*dst);
+  *dst = g_strdup (src);
+}
+
+
+/* Duplicate the string, but treat NULL as the empty string. */
+static char *
+file_prefs_strdup (char *src)
+{
+  return g_strdup (src == NULL ? "" : src);
+}
+
+/* Compare two strings, but treat NULL as the empty string. */
+static int
+file_prefs_strcmp (char *src1, char *src2)
+{
+  return strcmp (src1 == NULL ? "" : src1,
+		   src2 == NULL ? "" : src2);
+}
+
 static void
 file_prefs_ok_callback (GtkWidget *widget,
 			GtkWidget *dlg)
@@ -548,66 +635,71 @@
     update = g_list_append (update, "cubic-interpolation");
   if (confirm_on_close != old_confirm_on_close)
     update = g_list_append (update, "confirm-on-close");
+  if (default_width != old_default_width ||
+      default_height != old_default_height)
+    update = g_list_append (update, "default-image-size");
+  if (default_type != old_default_type)
+    update = g_list_append (update, "default-image-type");
   if (preview_size != old_preview_size)
     update = g_list_append (update, "preview-size");
   if (transparency_type != old_transparency_type)
     update = g_list_append (update, "transparency-type");
   if (transparency_size != old_transparency_size)
     update = g_list_append (update, "transparency-size");
-  if (old_stingy_memory_use != stingy_memory_use)
+  if (edit_stingy_memory_use != stingy_memory_use)
     {
       update = g_list_append (update, "stingy-memory-use");
-      stingy_memory_use = old_stingy_memory_use;
+      stingy_memory_use = edit_stingy_memory_use;
     }
-  if (old_tile_cache_size != tile_cache_size)
+  if (edit_tile_cache_size != tile_cache_size)
     {
       update = g_list_append (update, "tile-cache-size");
-      tile_cache_size = old_tile_cache_size;
+      tile_cache_size = edit_tile_cache_size;
     }
-  if (old_install_cmap != install_cmap)
+  if (edit_install_cmap != install_cmap)
     {
       update = g_list_append (update, "install-colormap");
-      install_cmap = old_install_cmap;
+      install_cmap = edit_install_cmap;
     }
-  if (old_cycled_marching_ants != cycled_marching_ants)
+  if (edit_cycled_marching_ants != cycled_marching_ants)
     {
       update = g_list_append (update, "colormap-cycling");
-      cycled_marching_ants = old_cycled_marching_ants;
+      cycled_marching_ants = edit_cycled_marching_ants;
     }
-  if (strcmp (temp_path, old_temp_path))
+  if (file_prefs_strcmp (temp_path, edit_temp_path))
     {
       update = g_list_append (update, "temp-path");
-      temp_path = old_temp_path;
+      temp_path = edit_temp_path;
     }
-  if (strcmp (swap_path, old_swap_path))
+  if (file_prefs_strcmp (swap_path, edit_swap_path))
     {
       update = g_list_append (update, "swap-path");
-      swap_path = old_swap_path;
+      swap_path = edit_swap_path;
     }
-  if (strcmp (brush_path, old_brush_path))
+  if (file_prefs_strcmp (brush_path, edit_brush_path))
     {
       update = g_list_append (update, "brush-path");
-      brush_path = old_brush_path;
+      brush_path = edit_brush_path;
     }
-  if (strcmp (pattern_path, old_pattern_path))
+  if (file_prefs_strcmp (pattern_path, edit_pattern_path))
     {
       update = g_list_append (update, "pattern-path");
-      pattern_path = old_pattern_path;
+      pattern_path = edit_pattern_path;
     }
-  if (strcmp (palette_path, old_palette_path))
+  if (file_prefs_strcmp (palette_path, edit_palette_path))
     {
       update = g_list_append (update, "palette-path");
-      palette_path = old_palette_path;
+      palette_path = edit_palette_path;
     }
-  if (strcmp (plug_in_path, old_plug_in_path))
+  if (file_prefs_strcmp (plug_in_path, edit_plug_in_path))
     {
       update = g_list_append (update, "plug-in-path");
-      plug_in_path = old_plug_in_path;
+      plug_in_path = edit_plug_in_path;
     }
-  if (strcmp (gradient_path, old_gradient_path))
+  if (file_prefs_strcmp (gradient_path, edit_gradient_path))
     {
       update = g_list_append (update, "gradient-path");
-      gradient_path = old_gradient_path;
+      gradient_path = edit_gradient_path;
     }
   save_gimprc (&update, &remove);
 
@@ -673,6 +765,18 @@
       gdisplays_expose_full ();
       gdisplays_flush ();
     }
+
+  edit_stingy_memory_use = old_stingy_memory_use;
+  edit_tile_cache_size = old_tile_cache_size;
+  edit_install_cmap = old_install_cmap;
+  edit_cycled_marching_ants = old_cycled_marching_ants;
+  file_prefs_strset (&edit_temp_path, old_temp_path);
+  file_prefs_strset (&edit_swap_path, old_swap_path);
+  file_prefs_strset (&edit_brush_path, old_brush_path);
+  file_prefs_strset (&edit_pattern_path, old_pattern_path);
+  file_prefs_strset (&edit_palette_path, old_palette_path);
+  file_prefs_strset (&edit_plug_in_path, old_plug_in_path);
+  file_prefs_strset (&edit_gradient_path, old_gradient_path);
 }
 
 static void
@@ -733,12 +837,12 @@
 
 static void
 file_prefs_string_callback (GtkWidget *widget,
-			  gpointer   data)
+			    gpointer   data)
 {
   gchar **val;
 
   val = data;
-  *val = gtk_entry_get_text (GTK_ENTRY (widget));
+  file_prefs_strset (val, gtk_entry_get_text (GTK_ENTRY (widget)));
 }
 
 void
@@ -793,17 +897,16 @@
   };
   struct {
     char *label;
-    char *path;
     char **mpath;
   } dirs[] =
     {
-      {"Temp dir:", temp_path, &old_temp_path},
-      {"Swap dir:", swap_path, &old_swap_path},
-      {"Brushes dir:", brush_path, &old_brush_path},
-      {"Gradients dir:", gradient_path, &old_gradient_path},
-      {"Patterns dir:", pattern_path, &old_pattern_path},
-      {"Palette dir:", palette_path, &old_palette_path},
-      {"Plug-in dir:", plug_in_path, &old_plug_in_path}
+      {"Temp dir:", &edit_temp_path},
+      {"Swap dir:", &edit_swap_path},
+      {"Brushes dir:", &edit_brush_path},
+      {"Gradients dir:", &edit_gradient_path},
+      {"Patterns dir:", &edit_pattern_path},
+      {"Palette dir:", &edit_palette_path},
+      {"Plug-in dir:", &edit_plug_in_path}
     };
     struct {
       char *label;
@@ -823,6 +926,22 @@
 
   if (!prefs_dlg)
     {
+      if (edit_temp_path == NULL)
+	{
+	  /* first time dialog is opened - copy config vals to edit
+             variables. */
+	  edit_temp_path = file_prefs_strdup (temp_path);	
+	  edit_swap_path = file_prefs_strdup (swap_path);
+	  edit_brush_path = file_prefs_strdup (brush_path);
+	  edit_pattern_path = file_prefs_strdup (pattern_path);
+	  edit_palette_path = file_prefs_strdup (palette_path);
+	  edit_plug_in_path = file_prefs_strdup (plug_in_path);
+	  edit_gradient_path = file_prefs_strdup (gradient_path);
+	  edit_stingy_memory_use = stingy_memory_use;
+	  edit_tile_cache_size = tile_cache_size;
+	  edit_install_cmap = install_cmap;
+	  edit_cycled_marching_ants = cycled_marching_ants;
+	}
       old_transparency_type = transparency_type;
       old_transparency_size = transparency_size;
       old_levels_of_undo = levels_of_undo;
@@ -835,17 +954,17 @@
       old_default_width = default_width;
       old_default_height = default_height;
       old_default_type = default_type;
-      old_stingy_memory_use = stingy_memory_use;
-      old_tile_cache_size = tile_cache_size;
-      old_install_cmap = install_cmap;
-      old_cycled_marching_ants = cycled_marching_ants;
-      old_temp_path = temp_path;
-      old_swap_path = swap_path;
-      old_brush_path = brush_path;
-      old_pattern_path = pattern_path;
-      old_palette_path = palette_path;
-      old_plug_in_path = plug_in_path;
-      old_gradient_path = gradient_path;
+      old_stingy_memory_use = edit_stingy_memory_use;
+      old_tile_cache_size = edit_tile_cache_size;
+      old_install_cmap = edit_install_cmap;
+      old_cycled_marching_ants = edit_cycled_marching_ants;
+      file_prefs_strset (&old_temp_path, edit_temp_path);
+      file_prefs_strset (&old_swap_path, edit_swap_path);
+      file_prefs_strset (&old_brush_path, edit_brush_path);
+      file_prefs_strset (&old_pattern_path, edit_pattern_path);
+      file_prefs_strset (&old_palette_path, edit_palette_path);
+      file_prefs_strset (&old_plug_in_path, edit_plug_in_path);
+      file_prefs_strset (&old_gradient_path, edit_gradient_path);
 
       prefs_dlg = gtk_dialog_new ();
       gtk_window_set_title (GTK_WINDOW (prefs_dlg), "Preferences");
@@ -1234,7 +1353,7 @@
  
           entry = gtk_entry_new ();
           gtk_widget_set_usize (entry, 25, 0);
-          gtk_entry_set_text (GTK_ENTRY (entry), dirs[i].path);
+          gtk_entry_set_text (GTK_ENTRY (entry), *(dirs[i].mpath));
 	  gtk_signal_connect (GTK_OBJECT (entry), "changed",
 			      (GtkSignalFunc) file_prefs_string_callback,
 			      dirs[i].mpath);

--- gtk+/gtk/gtkselection.c	Mon Nov  3 23:54:32 1997
+++ gtk+/gtk/gtkselection.c.new	Mon Nov  3 23:53:52 1997
@@ -205,8 +205,7 @@
 	  if (selection_info)
 	    {
 	      old_owner = selection_info->widget;
-	      current_selections = g_list_remove_link (current_selections,
-						       tmp_list);
+	      g_list_remove_link (current_selections, tmp_list);
 	      g_free (selection_info);
 	    }
 	}
@@ -297,8 +296,7 @@
 	      handler->data = data;
 	    }
 	  else
-	    selection_handlers = g_list_remove_link (selection_handlers,
-						     tmp_list);
+	    g_list_remove_link (selection_handlers, tmp_list);
 	  return;
 	}
       tmp_list = tmp_list->next;
@@ -345,7 +343,7 @@
     {
       if (((GtkIncrInfo *)tmp_list->data)->widget == widget)
 	{
-	  current_incrs = g_list_remove_link (current_incrs, tmp_list);
+	  g_list_remove_link (current_incrs, tmp_list);
 	}
       tmp_list = tmp_list->next;
     }
@@ -355,8 +353,7 @@
     {
       if (((GtkRetrievalInfo *)tmp_list->data)->widget == widget)
 	{
-	  current_retrievals = g_list_remove_link (current_retrievals,
-						   tmp_list);
+	  g_list_remove_link (current_retrievals, tmp_list);
 	}
       tmp_list = tmp_list->next;
     }
@@ -373,8 +370,7 @@
 	  gdk_selection_owner_set (NULL, 
 				   selection_info->selection,
 				   GDK_CURRENT_TIME, FALSE);
-	  current_selections = g_list_remove_link (current_selections,
-						   tmp_list);
+	  g_list_remove_link (current_selections, tmp_list);
 	  g_free (selection_info);
 	}
       
@@ -606,7 +602,7 @@
   if (tmp_list == NULL || selection_info->time > event->time)
     return TRUE;
 
-  current_selections = g_list_remove_link (current_selections, tmp_list);
+  g_list_remove_link (current_selections, tmp_list);
   g_free (selection_info);
 
   return TRUE;



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]