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

[Not Zed <notzed ximian com>]

Hi David,

A few minor things, but its looking pretty good.

 Z


> ===================================================================
> 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	26 Nov 2003 20:10:13 -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,40 @@
>  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, *path;
> +	const char *filename;
> +	
> +	filename = camel_mime_part_get_filename(t->data.part.part);
> +	if(filename == NULL)
> +		filename = _("Unknown");

This also needs to check for filename = "" (i.e. filename[0] == 0), and replace it with the unknown string.

Add a comment for the translators above filename = _("Unknown"), to describe what unknown this is, perhaps call it "Unnamed Image" or "Image" or something too.

Actually, you might also need/want to peek the mime type to create a suitable extension for the image file (.png, etc) if the name isn't set.

> +	path=g_build_filename(g_get_home_dir(), ".gnome2", "backgrounds", filename, NULL);
> +	
> +	if (em_utils_save_part_to_file(w, path, t->data.part.part)) {

Since this now saves to a filename based on the email itself, it needs to:
 - sanitise the filename, so it doesn't include /'s  (e.g. "../../.bashrc").
     use e_filename_make_safe, see its usage in em-utils.c:emu_get_save_filesel().
 - do some (automatic) smarts so it doesn't over-write something with the same name (e.g. add a " (n)" to the name if it clashes).
     I guess this logic could go into em_utils_save_path_to_file() since its intended for non-interactive, automatic, use.

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

i'd prefer

if (current_setting = gconf_client_get_string(...) != NULL
   && strcmp(current_setting, path) == 0) {
   set_string()
}

but the logic is identical

> +		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 "wallpaper"
> +		 * 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", "wallpaper", NULL);
> +		}

This logic should probably do:

if (  (current = get_string()) == NULL
    || strcmp(current, "none") == 0)
   set_string()

which is slightly different.

> +		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 -r1.8 em-utils.c
> --- mail/em-utils.c	19 Nov 2003 21:22:12 -0000	1.8
> +++ mail/em-utils.c	26 Nov 2003 20:10:16 -0000
> @@ -33,6 +33,7 @@
>  
>  #include <camel/camel-stream-fs.h>
>  #include <camel/camel-url-scanner.h>
> +#include <camel/camel-file-utils.h>
>  
>  #include <filter/filter-editor.h>
>  
> @@ -52,6 +53,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 +1384,57 @@
>  	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
> + * Creates all needed directories and overwrites 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;
> +	
> +	if (camel_mkdir(g_path_get_dirname(filename), 0754) == -1) {

You should just use 0777 here, the users umask setting will take care of the rest.

Also, g_path_get_dirname() returns a char * you need to free yourself, so you're leaking the dirname.

> +		e_notice (parent, GTK_MESSAGE_ERROR,
> +				 _("Cannot save to `%s'\n %s"), filename, g_strerror (errno));
> +		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;
> +		}
> +	}
> +	
> +	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));

should probably just "return 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 -r1.4 em-utils.h
> --- mail/em-utils.h	27 Oct 2003 21:31:19 -0000	1.4
> +++ mail/em-utils.h	26 Nov 2003 20:10:16 -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);