Re: Re: [evolution-patches] Bounty Hunt patch: Set wallpaper from

[David Moore <davmre bellsouth net>]

Thanks for the feedback. I've attached an updated patch which hopefully addresses all the issues. 

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

setting the path to "" causes GNOME to remove the current image and draw a gradient before drawing the next image. That's less efficient than simply drawing the next image, so I figured I might as well avoid it when possible.

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

I set "stretched" so that the image would appear when picture_options had been set to "none" (which is the default in several distros), because otherwise, the image would be there but GNOME wouldn't display it. I've changed it now so that it sets picture_options to "stretched" if "none" had been set, but otherwise leaves it alone.

> 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).

I've added an error message to the other test, so it's consistant. I'm leaving both tests in place because it's possible (if .background already exists as a directory, unlikely as that may be) for the 'stat' test to fail while the 'access'test doesn't. That particular bit of code was copied from emu_can_save(), so the same problem also exists there. I didn't change it, but someone might want to.

> 
> From: Not Zed <notzed ximian com>
> Date: 2003/11/24 Mon PM 04:22:15 EST
> To: David Moore <davmre bellsouth net>
> CC: evolution-patches lists ximian com
> Subject: Re: [evolution-patches] Bounty Hunt patch: Set wallpaper from
> 	mailer (mail)
> 
> 
> 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);
> 
> 

Index: mail/ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
retrieving revision 1.2896
diff -u -p -r1.2896 ChangeLog
--- mail/ChangeLog	23 Nov 2003 17:02:09 -0000	1.2896
+++ mail/ChangeLog	25 Nov 2003 00:45:10 -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 -p -r1.7 em-popup.c
--- mail/em-popup.c	12 Nov 2003 21:12:59 -0000	1.7
+++ mail/em-popup.c	25 Nov 2003 00:45:14 -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,36 @@ emp_part_popup_saveas(GtkWidget *w, EMPo
 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 *current_setting;
+	char *path;
+	
+	path=g_build_filename(g_get_home_dir(), "Desktop", ".background", NULL);
+	
+	if (em_utils_save_part_to_file(w, path, t->data.part.part)) {
+		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 ((current_setting = gconf_client_get_string(gconf, "/desktop/gnome/background/picture_filename", NULL)) != NULL) {
+			if (strcmp(current_setting, path) == 0)
+				gconf_client_set_string(gconf, "/desktop/gnome/background/picture_filename", "", NULL);
+		}
+		g_free(current_setting);
+		gconf_client_set_string(gconf, "/desktop/gnome/background/picture_filename", path, NULL);
+		
+		/* if GNOME currently doesn't display a picture, set to "stretched"
+		 * display mode, otherwise leave it alone */
+		if ((current_setting = gconf_client_get_string(gconf, "/desktop/gnome/background/picture_options", NULL)) != NULL) {
+			if (strcmp(current_setting, "none") == 0)
+				gconf_client_set_string(gconf, "/desktop/gnome/background/picture_options", "stretched", NULL);
+		}
+		g_free(current_setting);
+		
+		g_object_unref(gconf);
+	}
+	
+	g_free(path);
 }
 
 static void
Index: mail/em-utils.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/em-utils.c,v
retrieving revision 1.8
diff -u -p -r1.8 em-utils.c
--- mail/em-utils.c	19 Nov 2003 21:22:12 -0000	1.8
+++ mail/em-utils.c	25 Nov 2003 00:45:14 -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,52 @@ em_utils_save_part(GtkWidget *parent, co
 	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 (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;
+		}
+	}
+	
+	if (stat (filename, &st) != -1 && !S_ISREG (st.st_mode)) {
+		e_notice (parent, GTK_MESSAGE_ERROR,
+				 _("Error: '%s' exists and is not a regular file"), filename);
+		return FALSE;
+	}
+	
+
+	/* FIXME: This doesn't handle default charsets */
+	mail_msg_wait(mail_save_part(part, filename, emu_save_part_done, &done));
+	
+	if (!done) {
+		/* mail_save_part should popup an error box automagically */
+		return FALSE;
+	}
+	
+	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 -p -r1.4 em-utils.h
--- mail/em-utils.h	27 Oct 2003 21:31:19 -0000	1.4
+++ mail/em-utils.h	25 Nov 2003 00:45:14 -0000
@@ -86,6 +86,7 @@ void em_utils_reply_to_message_by_uid (s
 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);