|
Hello, Jeffrey. Maybe you have misunderstood me because of my poor expressing. :) I will raise my points with two parts of words below. Jeffrey Stedfast wrote:
En, thank you Jeffrey, I should pay more attation to the coding style.This is the dialog for selecting the file to save. If stat() return -1, it means that the file described with filename doesn't exist or something else. What we should do is , to keep the dialog open when the file described with filename exists and it is a directory indeed . So I prefer this one: char *pathname = g_path_get_dirname (filename); // get it's pathdir if (stat (pathname, &st) == -1) // if the pathdir doestn't exist, then ok doesn't work return TRUE; else if (! S_ISDIR (st.st_mode)) // if the pathdir isn't a dir, then ok doesn't work return TRUE; else if (stat (filename, &st) == -1) // if the file doesn't exist, then ok works return FALSE; else if (! S_ISREG (st.st_mode)) // if the file isn't a regular file, then ok doesn't work return TRUE; else return FALSE; // else, ok works Why I write so many creteria is to handle the case of "incorrect filename" and "file doesn't exist" separately. For example, I can leave the "~/does_not_exist" filename to the upper caller to process, but I cannot leave the "~/does_not_exist/a.msg" filename to the upper caller to process. And I don't like to put the criteria into one instrument like "stat()==0 && S_ISDIR()", I think it isn't clear enough and the result relies on the compiler's feature. Or you think it's not necessary to think about so much? Again, this dialog is for selecting the file to save. I think it's no need to save a mail's content to more than one files. And if it is at last, at least we should change the title to correct words. Such we should change the origin instrument from " run_selector(composer, _("Attach file(s)"), TRUE, NULL); " to " run_selector(composer, title, FALSE, NULL); " Is that right, or the whole point is to save to serveral files at the same time??? Thank you for pointing out my fault. Please review the patch. Best Regards Charles Zhang |
Index: composer/e-msg-composer-select-file.c
===================================================================
RCS file: /cvs/gnome/evolution/composer/e-msg-composer-select-file.c,v
retrieving revision 1.25
diff -u -p -r1.25 e-msg-composer-select-file.c
--- composer/e-msg-composer-select-file.c 2 Apr 2003 18:51:01 -0000 1.25
+++ composer/e-msg-composer-select-file.c 24 Sep 2003 15:31:49 -0000
@@ -38,6 +38,27 @@
#include "e-msg-composer-select-file.h"
+static gboolean
+ok_press_callback (GtkWidget *okbutton,
+ GdkEventButton *event,
+ GtkFileSelection *selection)
+{
+ char *filename = gtk_file_selection_get_filename (selection);
+ char *pathname = g_path_get_dirname (filename);
+ struct stat st;
+
+ if (stat (pathname, &st) == -1)
+ return TRUE;
+ else if (! S_ISDIR (st.st_mode))
+ return TRUE;
+ else if (stat (filename, &st) == -1)
+ return FALSE;
+ else if (! S_ISREG (st.st_mode))
+ return TRUE;
+ else
+ return FALSE;
+}
+
static GtkFileSelection *
run_selector(EMsgComposer *composer, const char *title, int multi, gboolean *showinline_p)
{
@@ -52,6 +73,10 @@ run_selector(EMsgComposer *composer, con
gnome_window_icon_set_from_file((GtkWindow *)selection, EVOLUTION_DATADIR "/images/evolution/compose-message.png");
gtk_file_selection_set_select_multiple((GtkFileSelection *)selection, multi);
+ g_signal_connect (((GtkFileSelection *) selection)->ok_button,
+ "button_press_event", G_CALLBACK (ok_press_callback),
+ (gpointer) selection);
+
/* restore last path used */
path = g_object_get_data((GObject *)composer, "attach_path");
if (path == NULL) {
@@ -98,6 +123,9 @@ e_msg_composer_select_file (EMsgComposer
{
GtkFileSelection *selection;
char *name = NULL;
+
+ /* FIXME: the next instrument should be "run_selector(composer, title, FALSE, NULL)",
+ otherwise this dialog's title is wrong and more than one file can be selected. */
selection = run_selector(composer, _("Attach file(s)"), TRUE, NULL);
if (selection) {