GNOME.org
 

Re: [evolution-patches] Re: [Evolution-hackers] Possibility to save all attachments

er, sorry. yea - looks good. I thought I commented on it earlier today
but apparently I didn't :-)

Jeff

On Thu, 2003-07-31 at 00:05, Harry Lu wrote:
> Jeff,
>    Would you please review the patch again? 
>    Thanks! 
>       Harry 
>     
> 
> Harry Lu wrote:
> > Anna, Jeff and others, 
> >    Thanks for the review. I modified my patch and attached it here. 
> >    For the first problem, since we are using GtkFileSelection widget
> > of GTK to let user select directory, it is very hard for us to
> > change its behavior.  You have to double click to make the selection
> > work. This is also the normal behavior for the some File selection
> > dialogs. At least I am used to it. Anyway, I can hide the selection
> > text entry so that user can only see the selection label.  So the
> > selection label will be the selection user should refer to. You can
> > apply the patch to see the result. If you don't like it, I can show
> > the entry again. 
> >    For the second, yes, my fault. English is not my native language,
> > so sometimes I make mistakes:( I had changed the menu item and the
> > dialog title according to your suggestions. 
> >    For the third, I agree with you :) 
> >   
> > Jeff, would you please review the patch again? 
> >    Thanks! 
> >       Harry 
> >   
> > anna ximian com wrote: 
> > 
> > > Hi Harry, Ettore and others, 
> > > 
> > > Thank you for your work on this patch! I have read many, many bug
> > > reports 
> > > from people who want to use this feature, and I am happy that we
> > > will be 
> > > able to offer it to them soon. Nice work. 
> > > 
> > > For the most part, I think that this patch seems good from a UI 
> > > perspective. There are a few little things that bother me. They
> > > are: 
> > > 
> > > 1. On my machine, it is difficult to tell when a directory has
> > > been 
> > > selected. Look at the following screenshot: 
> > > http://primates.ximian.com/~anna/save-all.png . Have I succeeded
> > > in 
> > > selecting the "Documents" directory? I cannot tell. If I press the
> > > "OK" 
> > > button, what will happen? Will my attachments be saved in
> > > "Desktop", or in 
> > > "Documents" ? 
> > > 
> > > I'd really like it if we could make the selected directory -- in
> > > this 
> > > case, "Documents", appear in the text entry beneath the
> > > "Selection" label. 
> > > Yes, I know it is a directroy, not a file, and that file names
> > > usually go 
> > > in the "Selection" entry. To be clear, in this case, that would
> > > mean that 
> > > "Documents" would be shown in the text entry.  What do you think?
> > > I think 
> > > if it is possible for the name of the selected directory to be
> > > shown 
> > > there, then, that would be helpful in eliminating user confusion
> > > about 
> > > precisely where her files will be saved. 
> > > 
> > > 2. Cosmetic Stuff: The plural form of the singular noun
> > > "attachment" is 
> > > "attachments"; you tended to use "attachment", which is incorrect.
> > > The 
> > > labels you used in your patch should be updated as follows: the
> > > name of 
> > > the context menu item should be "Save all attachments..." (note
> > > that "all" 
> > > is not capitalized). 
> > > 
> > > The title of the file selector should be shortened a bit, to be
> > > more 
> > > consistent with the window title naming conventions used in the
> > > rest of 
> > > Evolution; "Select Directory for Attachments" would be more
> > > appropriate. 
> > > What do you think of that? Just to let you know what the HIG says
> > > on the 
> > > matter, see 
> > > http://developer.gnome.org/projects/gup/hig/1.0/windows.html#window-props-titles
> > > . The gist of the guidelines is that window titles should
> > > distinguish the 
> > > window from other windows, but should not provide unnecessary
> > > information. 
> > > 
> > > 3. As for making other places in the UI for this option, I agree
> > > with you; 
> > > figuring out the places where we should make this available is
> > > another 
> > > bug. Specifically, it is this bug: 
> > > http://bugzilla.ximian.com/show_bug.cgi?id=47048 . Let's discuss
> > > the 
> > > possibilities for how to present these options there. 
> > > 
> > > Thank you, 
> > > Anna 
> > > 
> > > 
> > > 
> > > 
> > > _______________________________________________ 
> > > Evolution-patches mailing list 
> > > Evolution-patches lists ximian com
> > > http://lists.ximian.com/mailman/listinfo/evolution-patches
> > >   
> > > 
> > 
> > ____________________________________________________________________
> > Index: evolution/mail/ChangeLog
> > ===================================================================
> > RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
> > retrieving revision 1.2780
> > diff -u -r1.2780 ChangeLog
> > --- evolution/mail/ChangeLog	28 Jul 2003 16:09:14 -0000	1.2780
> > +++ evolution/mail/ChangeLog	30 Jul 2003 05:42:50 -0000
> > @@ -1,3 +1,18 @@
> > +2003-07-28  Harry Lu  <harry lu sun com>
> > +
> > +	*Fix for bug #6951
> > +
> > +	* mail-display.c (launch_cb): Bypass the new added menu item.
> > +	(save_all_parts_cb): New function. Do the real save-all work.
> > +	(save_all_parts): New function. Get the directory to save to.
> > +	(save_all_cb): New function. The call-back function for the new 
> > +	added menu item.
> > +	(pixmap_press): Add the new menu item "Save All Attachment...".
> > +	(ptr_array_free_notify): A simple wrapper function to free the
> > +	pointer array.
> > +	(do_attachment_header): Save attachment pointer in an array for 
> > +	"Save All Attachment" use.
> > +
> >  2003-07-28  Jeffrey Stedfast  <fejj ximian com>
> >  
> >  	* mail-crypto.c: Removed smime functions as they were stale.
> > Index: evolution/mail/mail-display.c
> > ===================================================================
> > RCS file: /cvs/gnome/evolution/mail/mail-display.c,v
> > retrieving revision 1.287
> > diff -u -r1.287 mail-display.c
> > --- evolution/mail/mail-display.c	23 Jul 2003 15:01:26 -0000	1.287
> > +++ evolution/mail/mail-display.c	30 Jul 2003 05:42:56 -0000
> > @@ -375,9 +375,11 @@
> >  	
> >  	/* Yum. Too bad EPopupMenu doesn't allow per-item closures. */
> >  	children = gtk_container_get_children (GTK_CONTAINER (widget->parent));
> > -	g_return_if_fail (children != NULL && children->next != NULL && children->next->next != NULL);
> > -	
> > -	for (c = children->next->next, apps = handler->applications; c && apps; c = c->next, apps = apps->next) {
> > +	/* We need to bypass the first 2 menu items */
> > +	g_return_if_fail (children != NULL && children->next != NULL 
> > +		&& children->next->next != NULL && children->next->next->next != NULL);
> > +
> > +	for (c = children->next->next->next, apps = handler->applications; c && apps; c = c->next, apps = apps->next) {
> >  		if (c->data == widget)
> >  			break;
> >  	}
> > @@ -435,7 +437,107 @@
> >  	mail_display_queue_redisplay (md);
> >  }
> >  
> > -static gboolean
> > +static void
> > +save_all_parts_cb (GtkWidget *widget, gpointer user_data)
> > +{
> > +	GtkFileSelection *dir_select = (GtkFileSelection *) 
> > +		gtk_widget_get_ancestor (widget, GTK_TYPE_FILE_SELECTION);
> > +	const char *filename;
> > +	char *save_filename, *dir;
> > +	struct stat st;
> > +	int i;
> > +	GPtrArray *attachment_array;
> > +	CamelMimePart *part;
> > +	GConfClient *gconf;
> > +
> > +	gtk_widget_hide (GTK_WIDGET (dir_select));
> > +
> > +	/* Get the selected directory name */
> > +	filename = gtk_file_selection_get_filename (dir_select);
> > +	if (stat (filename, &st) == -1 || !S_ISDIR (st.st_mode)) {
> > +		GtkWidget *dialog;
> > +
> > +		dialog = gtk_message_dialog_new (NULL, 0, GTK_MESSAGE_ERROR, GTK_BUTTONS_CLOSE,
> > +			_("%s is not a valid directory name."), filename);
> > +
> > +		/* FIXME: this should be async */
> > +		gtk_dialog_run ((GtkDialog *) dialog);
> > +		gtk_widget_destroy (dialog);
> > +		gtk_widget_destroy (GTK_WIDGET (dir_select));
> > +		return;
> > +	} else {
> > +		dir = g_strdup (filename);
> > +	}
> > +						
> > +	/* Now save the attachment one by one */
> > +	attachment_array = (GPtrArray *)user_data;
> > +	for (i = 0; i < attachment_array->len; i++) {
> > +		part = g_ptr_array_index (attachment_array, i);
> > +		save_filename = make_safe_filename (dir, part);
> > +		write_data_to_file (part, save_filename, FALSE);
> > +		g_free (save_filename);
> > +	}
> > +							
> > +	/* preserve the pathname */
> > +	gconf = mail_config_get_gconf_client ();
> > +	gconf_client_set_string (gconf, "/apps/evolution/mail/save_dir", dir, NULL);
> > +	g_free (dir);
> > +							
> > +	gtk_widget_destroy (GTK_WIDGET (dir_select));
> > +}
> > +
> > +static void 
> > +save_all_parts (GPtrArray *attachment_array)
> > +{
> > +	GtkFileSelection *dir_select;
> > +	char *dir, *home, *dir2;
> > +	GConfClient *gconf;
> > +
> > +	g_return_if_fail (attachment_array !=  NULL);
> > +
> > +	home = getenv ("HOME");
> > +	gconf = mail_config_get_gconf_client ();
> > +	dir = gconf_client_get_string (gconf, "/apps/evolution/mail/save_dir", NULL);
> > +	dir = dir ? dir : (home ? g_strdup (home) : g_strdup (""));
> > +
> > +	/* Make sure dir2 has a '/' as its tail */
> > +	dir2 = g_strdup_printf ("%s/", dir);
> > +	g_free (dir);
> > +
> > +	dir_select = GTK_FILE_SELECTION (
> > +		gtk_file_selection_new (_("Select Directory for Attachments")));
> > +	gtk_file_selection_set_filename (dir_select, dir2);
> > +	gtk_widget_set_sensitive (dir_select->file_list, FALSE);
> > +	gtk_widget_hide (dir_select->selection_entry);
> > +	g_free (dir2);
> > +
> > +	g_signal_connect (dir_select->ok_button, "clicked", 
> > +		G_CALLBACK (save_all_parts_cb), attachment_array);
> > +	g_signal_connect_swapped (dir_select->cancel_button,
> > +		"clicked",
> > +		G_CALLBACK (gtk_widget_destroy),
> > +		dir_select);
> > +
> > +	gtk_widget_show (GTK_WIDGET (dir_select));
> > +}
> > +		
> > +static void
> > +save_all_cb (GtkWidget *widget, gpointer user_data)
> > +{
> > +	MailDisplay *md = g_object_get_data (user_data, "MailDisplay");
> > +	GPtrArray *attachment_array;
> > +			
> > +	if (md == NULL) {
> > +		g_warning ("No MailDisplay!");
> > +		return;
> > +	}
> > +
> > +	attachment_array = g_datalist_get_data (md->data, "attachment_array");
> > +	save_all_parts (attachment_array);
> > +}
> > +
> > +
> > +static gboolean 
> >  button_press (GtkWidget *widget, GdkEvent *event, CamelMimePart *part)
> >  {
> >  	MailDisplay *md;
> > @@ -475,12 +577,14 @@
> >  	EPopupMenu *menu;
> >  	GtkMenu *gtk_menu;
> >  	EPopupMenu save_item = E_POPUP_ITEM (N_("Save Attachment..."), G_CALLBACK (save_cb), 0);
> > +	EPopupMenu save_all_item = E_POPUP_ITEM (N_("Save all attachments..."), G_CALLBACK (save_all_cb), 0);
> >  	EPopupMenu view_item = E_POPUP_ITEM (N_("View Inline"), G_CALLBACK (inline_cb), 2);
> >  	EPopupMenu open_item = E_POPUP_ITEM (N_("Open in %s..."), G_CALLBACK (launch_cb), 1);
> >  	MailDisplay *md;
> >  	CamelMimePart *part;
> >  	MailMimeHandler *handler;
> >  	int mask = 0, i, nitems;
> > +	int current_item = 0;
> >  	
> >  	if (event->type == GDK_BUTTON_PRESS) {
> >  #ifdef USE_OLD_DISPLAY_STYLE
> > @@ -508,17 +612,23 @@
> >  	handler = mail_lookup_handler (g_object_get_data ((GObject *) widget, "mime_type"));
> >  	
> >  	if (handler && handler->applications)
> > -		nitems = g_list_length (handler->applications) + 2;
> > +		nitems = g_list_length (handler->applications) + 3;
> >  	else
> > -		nitems = 3;
> > +		nitems = 4;
> >  	menu = g_new0 (EPopupMenu, nitems + 1);
> >  	
> >  	/* Save item */
> > -	memcpy (&menu[0], &save_item, sizeof (menu[0]));
> > -	menu[0].name = _(menu[0].name);
> > -	
> > +	memcpy (&menu[current_item], &save_item, sizeof (menu[current_item]));
> > +	menu[current_item].name = g_strdup (_(menu[current_item].name));
> > +	current_item++;
> > +
> > +	/* Save All item */
> > +	memcpy (&menu[current_item], &save_all_item, sizeof (menu[current_item]));
> > +	menu[current_item].name = g_strdup (_(menu[current_item].name));
> > +	current_item++;
> > +	 
> >  	/* Inline view item */
> > -	memcpy (&menu[1], &view_item, sizeof (menu[1]));
> > +	memcpy (&menu[current_item], &view_item, sizeof (menu[current_item]));
> >  	if (handler && handler->builtin) {
> >  		md = g_object_get_data ((GObject *) widget, "MailDisplay");
> >  		
> > @@ -536,15 +646,16 @@
> >  					name = prop->v._u.value_string;
> >  				else
> >  					name = "bonobo";
> > -				menu[1].name = g_strdup_printf (_("View Inline (via %s)"), name);
> > +				menu[current_item].name = g_strdup_printf (_("View Inline (via %s)"), name);
> >  			} else
> > -				menu[1].name = g_strdup (_(menu[1].name));
> > +				menu[current_item].name = g_strdup (_(menu[current_item].name));
> >  		} else
> > -			menu[1].name = g_strdup (_("Hide"));
> > +			menu[current_item].name = g_strdup (_("Hide"));
> >  	} else {
> > -		menu[1].name = g_strdup (_(menu[1].name));
> > +		menu[current_item].name = g_strdup (_(menu[current_item].name));
> >  		mask |= 2;
> >  	}
> > +	current_item++;
> >  	
> >  	/* External views */
> >  	if (handler && handler->applications) {
> > @@ -553,14 +664,15 @@
> >  		int i;
> >  		
> >  		apps = handler->applications;
> > -		for (i = 2; i < nitems; i++, apps = apps->next) {
> > +		for (i = current_item; i < nitems; i++, apps = apps->next) {
> >  			app = apps->data;
> >  			memcpy (&menu[i], &open_item, sizeof (menu[i]));
> >  			menu[i].name = g_strdup_printf (_(menu[i].name), app->name);
> > +			current_item++;
> >  		}
> >  	} else {
> > -		memcpy (&menu[2], &open_item, sizeof (menu[2]));
> > -		menu[2].name = g_strdup_printf (_(menu[2].name), _("External Viewer"));
> > +		memcpy (&menu[current_item], &open_item, sizeof (menu[current_item]));
> > +		menu[current_item].name = g_strdup_printf (_(menu[current_item].name), _("External Viewer"));
> >  		mask |= 1;
> >  	}
> >  	
> > @@ -983,6 +1095,12 @@
> >  	}
> >  }
> >  
> > +/* This is a wrapper function */
> > +void ptr_array_free_notify (gpointer array)
> > +{
> > +	g_ptr_array_free ((GPtrArray *) array, TRUE);
> > +}
> > +
> >  static gboolean
> >  do_attachment_header (GtkHTML *html, GtkHTMLEmbedded *eb,
> >  		      CamelMimePart *part, MailDisplay *md)
> > @@ -990,6 +1108,7 @@
> >  	GtkWidget *button, *mainbox, *hbox, *arrow, *popup;
> >  	MailMimeHandler *handler;
> >  	struct _PixbufLoader *pbl;
> > +	GPtrArray *attachment_array;
> >  	
> >  	pbl = g_new0 (struct _PixbufLoader, 1);
> >  	if (strncasecmp (eb->type, "image/", 6) == 0) {
> > @@ -1059,6 +1178,19 @@
> >  	g_object_set_data ((GObject *) popup, "MailDisplay", md);
> >  	g_object_set_data ((GObject *) popup, "CamelMimePart", part);
> >  	g_object_set_data_full ((GObject *) popup, "mime_type", g_strdup (eb->type), (GDestroyNotify) g_free);
> > +
> > +	/* Save attachment pointer in an array for "save all attachment" use */	
> > +	attachment_array = g_datalist_get_data (md->data, "attachment_array");
> > +	if (!attachment_array) {
> > +		attachment_array = g_ptr_array_new ();
> > +		g_datalist_set_data_full (md->data, "attachment_array", 
> > +			attachment_array, (GDestroyNotify) ptr_array_free_notify);
> > +	}
> > +	/* Since the attachment pointer might have been added to the array before,
> > +	remove it first anyway to avoide duplication */
> > +	g_ptr_array_remove (attachment_array, part);
> > +	g_ptr_array_add (attachment_array, part);
> > +		 
> >  	
> >  	g_signal_connect (popup, "button_press_event", G_CALLBACK (pixmap_press), md->scroll);
> >  	g_signal_connect (popup, "key_press_event", G_CALLBACK (pixmap_press), md->scroll);
> >   
-- 
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
fejj ximian com  - www.ximian.com
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]