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

[Jeffrey Stedfast <fejj ximian com>]

On Fri, 2003-07-25 at 03:41, Harry Lu wrote:
> Jeff, NotZed and Anna,
>     I attatched the patch for HEAD here. Please review it.
>     Thanks!
>        Harry
> 
> Ettore Perazzoli wrote:
> > On Thu, 2003-07-24 at 02:33, Kósa Ferenc wrote:
> >   
> > > I use evolution since ver 1.0.2, and I am very satisfied with it. But
> > > would somebody be so kind to implement the function of saving all
> > > attachments in one place?
> > >     
> > This list is not for feature requests, please use evolution ximian com
> > for that.
> > 
> > But aside from this, we had a patch from Harry Lu which implemented this
> > functionality a while ago.  Can we get this reviewed and committed to
> > HEAD now?
> > 
> > http://lists.ximian.com/archives/public/evolution-patches/2003-June/001760.html
> >  
> > (Anna should review the UI change of course.)
> > 
> > -- Ettore
> > _______________________________________________
> > evolution-hackers maillist  -  evolution-hackers lists ximian com
> > http://lists.ximian.com/mailman/listinfo/evolution-hackers
> > 
> > 
> >   
> 
> 
> ______________________________________________________________________
> Index: evolution/mail/ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
> retrieving revision 1.2778
> diff -u -r1.2778 ChangeLog
> --- evolution/mail/ChangeLog	23 Jul 2003 17:37:29 -0000	1.2778
> +++ evolution/mail/ChangeLog	25 Jul 2003 07:35:07 -0000
> @@ -1,3 +1,18 @@
> +2003-07-25  Harry Lu  <harry lu sun com>
> +
> +	*Fix for bug #6951
> +
> +	* mail-display.c (launch_cb): Bypass the new added menu item.
> +	(save_all_data_cb): New function. Do the real save-all work.
> +	(save_all_part): 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-23  Jeffrey Stedfast  <fejj ximian com>
>  
>  	* mail-format.c (mail_format_data_wrapper_write_to_stream): Revert
> 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	25 Jul 2003 07:35:10 -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,112 @@
>  	mail_display_queue_redisplay (md);
>  }
>  
> -static gboolean
> +static void
> +save_all_data_cb (GtkWidget *widget, gpointer user_data)

change this to save_all_parts_cb()

> +{
> +	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) {
> +		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 if (S_ISDIR (st.st_mode)) {
> +		dir = g_strdup (filename);
> +	} else {
> +		dir = g_path_get_dirname (filename);
> +	}

hmmm, I would change this to be:

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);	
}

also, please fix the FIXME :-)

> +						
> +	/* 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_part (GPtrArray *attachment_array)

change this to save_all_parts()

save_all_part doesn't make any sense in english...

> +{
> +	GtkFileSelection *dir_select;
> +	char *dir, *home, *p, *dir2;
> +	GConfClient *gconf;
> +
> +	g_return_if_fail (attachment_array!= NULL);

spaces! spaces! they are free!

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 */
> +	p = dir + strlen(dir);

strlen (dir);

I just prefer spaces because it makes it easier for me to read. I'm
probably over anal about it, but...whatever :-)

> +	if (*p == '/')
> +		dir2 = g_strdup (dir);
> +	else
> +		dir2 = g_strdup_printf ("%s/", dir);

actually, instead of doing all this stuff - why not just do:

dir2 = g_strdup_printf ("%s/", dir);
g_free (dir);

also, we *may* need to use g_filename_from_utf8() before passing to
gtk_file_selection_set_filename()... at least that's what it seems to
required based on the docs at 

http://developer.gnome.org/doc/API/2.0/gtk/GtkFileSelection.html#gtk-file-selection-set-filename

> +
> +	dir_select = GTK_FILE_SELECTION (
> +		gtk_file_selection_new (_("Select a Directory to Save All Attachment")));
> +	gtk_file_selection_set_filename (dir_select, dir2);
> +	gtk_widget_set_sensitive (GTK_WIDGET(dir_select->file_list), FALSE);
> +	g_free (dir);
> +	g_free (dir2);
> +
> +	g_signal_connect (dir_select->ok_button, "clicked", 
> +		G_CALLBACK (save_all_data_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_part (attachment_array);
> +}
> +
> +
> +static gboolean 
>  button_press (GtkWidget *widget, GdkEvent *event, CamelMimePart *part)
>  {
>  	MailDisplay *md;
> @@ -475,12 +582,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 Attachment..."), 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 +617,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 +651,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 +669,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 +1100,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 +1113,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 +1183,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");

attachment_array = g_datalist_get_data (md->data, "attachment_array");

add a space :-)

> +	if (!attachment_array) {
> +		attachment_array = g_ptr_array_new();

attachment_array = g_ptr_array_new ();

add a space :-)

> +		g_datalist_set_data_full (md->data, "attachment_array", 
> +			attachment_array, (GDestroyNotify)ptr_array_free_notify);

attachment_array, (GDestroyNotify) ptr_array_free_notify);

add a space :-)

> +	}
> +	/* 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, (gpointer)part);
> +	g_ptr_array_add (attachment_array, (gpointer)part);

no need to cast these to gpointer, so please remove the cast

> +		 
>  	
>  	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