Re: [evolution-patches] Bounty Hunt patch: Set wallpaper from mailer (mail)

[Not Zed <notzed ximian com>]

Cool, although there are some minor nitpicks.

On Mon, 2003-11-24 at 15:03, David Moore wrote:

Hi,

http://bugzilla.gnome.org/show_bug.cgi?id=127514
patch: attached and at http://bugzilla.gnome.org/showattachment.cgi?attach_id=21744

This is a patch for CVS head, allowing the user to save an image attachment as the GNOME desktop background.

---

Index: mail/ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
retrieving revision 1.2896
diff -u -r1.2896 ChangeLog
--- mail/ChangeLog	23 Nov 2003 17:02:09 -0000	1.2896
+++ mail/ChangeLog	24 Nov 2003 02:51:36 -0000
@@ -1,3 +1,15 @@
+2003-11-22  David Moore  <davmre bellsouth net>
+
+	Bug #127514.
+
+	* em-popup.c (emp_part_popup_set_background): Implemented; change
+	the GNOME background image to an image attachment.
+	
+	* em-utils.c (emu_save_part_done): Created a prototype to the top
+	of the file, so it can be called from anywhere within
+	(em_utils_save_part_to_file): Added; save a message part to a 
+	specified file on disk.
+
 2003-11-22  Jeffrey Stedfast  <fejj ximian com>
 
 	* em-folder-tree-model.c (model_drag_data_received)
Index: mail/em-popup.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/em-popup.c,v
retrieving revision 1.7
diff -u -r1.7 em-popup.c
--- mail/em-popup.c	12 Nov 2003 21:12:59 -0000	1.7
+++ mail/em-popup.c	24 Nov 2003 02:51:37 -0000
@@ -23,6 +23,9 @@
 #include <camel/camel-mime-message.h>
 #include <camel/camel-string-utils.h>
 
+#include <gconf/gconf.h>
+#include <gconf/gconf-client.h>
+
 static void emp_standard_menu_factory(EMPopup *emp, EMPopupTarget *target, void *data);
 
 struct _EMPopupFactory {
@@ -601,8 +604,23 @@
 static void
 emp_part_popup_set_background(GtkWidget *w, EMPopupTarget *t)
 {
-	/* set as background ... */
-	printf("UNIMPLEMENTED: set background, but it would be cool, no?\n");
+	GConfClient *gconf;
+	char *path;
+	
+	path=g_strdup_printf("%s/Desktop/.background", g_get_home_dir());

this should use g_build_filename()

> +	if(em_utils_save_part_to_file(w, path, t->data.part.part)) {

good to have a space between if and (  and around '='.

> +		gconf = gconf_client_get_default();
> +		
> +		/* if the filename hasn't changed, blank the filename before 
> +		*  setting it so that gconf detects a change and updates it */
> +		if(strcmp(gconf_client_get_string(gconf, "/desktop/gnome/background/picture_filename", NULL), path)==0)

I think you'll need to check the return code of gconf_client_get_string for NULL, e.g. if there is a gconf error it could return NULL, and shouldn't crash.

perhaps it should just do 'set path "", set path filename', rather than having to check it first?

> +			gconf_client_set_string(gconf, "/desktop/gnome/background/picture_filename", "", NULL);
> +		
> +		gconf_client_set_string(gconf, "/desktop/gnome/background/picture_filename", path, NULL);
> +		gconf_client_set_string(gconf, "/desktop/gnome/background/picture_options", "stretched", NULL);

should we explictly set stretched, or just let it be done via the control panel?

> +		g_object_unref(gconf);
> +	}
>  }
you leak 'path'  :)
>  static void
> Index: mail/em-utils.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-utils.c,v
> retrieving revision 1.8
> diff -u -r1.8 em-utils.c
> --- mail/em-utils.c	19 Nov 2003 21:22:12 -0000	1.8
> +++ mail/em-utils.c	24 Nov 2003 02:51:40 -0000
> @@ -52,6 +52,7 @@
>  #include "em-format-quote.h"
>  
>  static EAccount *guess_account (CamelMimeMessage *message);
> +static void emu_save_part_done (CamelMimePart *part, char *name, int done, void *data);
>  
>  /**
>   * em_utils_prompt_user:
> @@ -1382,6 +1383,43 @@
>  	gtk_widget_show((GtkWidget *)filesel);
>  }
>  
> +/**
> + * em_utils_save_part_to_file:
> + * @parent: parent window
> + * @filename: filename to save to
> + * @part: part to save
> + * 
> + * Save a part's content to a specific file
> + * Will overwrite without prompting
> + *
> + * Returns %TRUE if saving succeeded, %FALSE otherwise
> + **/
> +gboolean 
> +em_utils_save_part_to_file(GtkWidget *parent, const char *filename, CamelMimePart *part) 
> +{
> +	int done;
> +	struct stat st;
> +	
> +	if (filename[0] == 0)
> +		return FALSE;
> +
> +	/* make sure we can actually save to it... */
> +	if (stat (filename, &st) != -1 && !S_ISREG (st.st_mode))
> +		return FALSE;
> +	
> +	if (access (filename, F_OK) == 0) {
> +		if (access (filename, W_OK) != 0) {
> +			e_notice (parent, GTK_MESSAGE_ERROR,
> +				 _("Cannot save to `%s'\n %s"), filename, g_strerror (errno));
> +			return FALSE;
> +		}
> +	}
there's some inconsistency above, in one case it pops up an error, in another it doesn't, you're testing basically the same thing twice too (i.e. if it exists and is a normal file, then if it exists and can be written to).

> +	/* FIXME: This doesn't handle default charsets */
> +	mail_msg_wait(mail_save_part(part, filename, emu_save_part_done, &done));
> +	
> +	return TRUE;
> +}
>  
>  struct _save_messages_data {
>  	CamelFolder *folder;
> Index: mail/em-utils.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-utils.h,v
> retrieving revision 1.4
> diff -u -r1.4 em-utils.h
> --- mail/em-utils.h	27 Oct 2003 21:31:19 -0000	1.4
> +++ mail/em-utils.h	24 Nov 2003 02:51:40 -0000
> @@ -86,6 +86,7 @@
>  void em_utils_post_reply_to_message_by_uid (struct _CamelFolder *folder, const char *uid);
>  
>  void em_utils_save_part(struct _GtkWidget *parent, const char *prompt, struct _CamelMimePart *part);
> +gboolean em_utils_save_part_to_file(struct _GtkWidget *parent, const char *filename, struct _CamelMimePart *part);
>  void em_utils_save_messages (struct _GtkWidget *parent, struct _CamelFolder *folder, GPtrArray *uids);
>  
>  void em_utils_flag_for_followup (struct _GtkWidget *parent, struct _CamelFolder *folder, GPtrArray *uids);