Re: [Fwd: Re: [evolution-patches] Patch for #ifdef'ed GtkFileChooser support]

[Jeffrey Stedfast <fejj ximian com>]

[commenting on mailer parts of the patch]

[composer]

I think I'd prefer it if run_selector() took a single 'flags' argument
that was a set of bit-flags.

run_selector (composer, title, SELECTOR_OPEN | SELECTOR_MULTI,
&show_inline);

or some such.

+        if (showinline_p) {
+                showinline = gtk_check_button_new_with_label
(_("Suggest automatic display of attachment"));
+                gtk_widget_show (showinline);
+               gtk_file_chooser_set_extra_widget (GTK_FILE_CHOOSER
(selection), showinline);
+        }

can you make sure that the indents here are all tabs? the way they are
offset from each other suggests that some are spaced over and some are
tabbed over.

in em-utils.c, you end up leaking memory:

        gconf = gconf_client_get_default();
-       dir = gdir = gconf_client_get_string(gconf,
"/apps/evolution/mail/save_dir", NULL);
+       dir = gconf_client_get_string(gconf,
"/apps/evolution/mail/save_dir", NULL);
        g_object_unref(gconf);
+
        if (dir == NULL)
                dir = g_get_home_dir();
 
        if (name && name[0]) {
-               realname = mname = g_strdup(name);
-               e_filename_make_safe(mname);
+               realname = g_strdup(name);
+               e_filename_make_safe(realname);
        } else {
-               realname = "/";
+               realname = NULL;
        }
 
-       filename = g_build_filename(dir, realname, NULL);
-       gtk_file_selection_set_filename(filesel, filename);
+#ifdef USE_GTKFILECHOOSER
+       gtk_file_chooser_set_current_folder (GTK_FILE_CHOOSER (filesel),
dir);
+
+       if (realname)
+               gtk_file_chooser_set_current_name (GTK_FILE_CHOOSER
(filesel), realname);
+#else
+       filename = g_build_filename(dir, G_DIR_SEPARATOR_S, realname,
NULL);
+       gtk_file_selection_set_filename(GTK_FILE_SELECTION (filesel),
filename);
        g_free(filename);
-       g_free(mname);
-       g_free (gdir);
+#endif
 
        return filesel;
 }

both realname and dir are leaked (except when dir is init'd using
g_get_home_dir). this is why there were 2 dir variables :)

@@ -398,7 +422,7 @@
 em_utils_save_part(GtkWidget *parent, const char *prompt, CamelMimePart
*part)
 {
        const char *name;
-       GtkFileSelection *filesel;
+       GtkWidget *filesel;
 
        name = camel_mime_part_get_filename(part);
        if (name == NULL) {
@@ -413,8 +437,8 @@
 
        filesel = emu_get_save_filesel(parent, prompt, name);
        camel_object_ref(part);
-       g_signal_connect(filesel, "response",
G_CALLBACK(emu_save_part_response), part);
-       gtk_widget_show((GtkWidget *)filesel);
+       g_signal_connect(G_OBJECT (filesel), "response",
G_CALLBACK(emu_save_part_response), part);

no need for this G_OBJECT() cast.

+       gtk_widget_show(filesel);
 }


I think the rest of composer/filter/mail changes look ok.

Jeff