Re: [evolution-patches] Fwd: Bug #127526 Possible Fix + Questions



On Thu, 2004-08-12 at 17:56 -0400, Tim Horton wrote:
> 
> 
> ______________________________________________________________________
> 
> I thought maybe I should send this here too...
> 
>         From: Tim Horton <rhorton16 adelphia net>
>         Date: August 11, 2004 9:51:22 PM EDT
>         To: evolution-hackers lists ximian com
>         Subject: Bug #127526 Possible Fix + Questions
>         
>         I have been working on Bug #127526, the "set Evolution as the
>         default mail client" bounty.
>         
>         I've got a very nicely working patch, with a "don't ask again"
>         feature which can be toggled either when the dialog appears,
>         or in the Mailer preferences pane.
>         
>         The message will only appear when Evolution is _not_ the
>         default mailer AND when Evolution is allowed to ask the user
>         again.
>         
>         One question I have : should I make future patches off of CVS
>         or of the latest development release? This one is based off of
>         the latest development release : 1.5.91. The only problem I
>         have with applying the patch to the CVS copy is the Changelog
>         (because of the 3 lines of context, etc...)
>         
>         I was able to apply this patch by changing into the evolution-
>         1.5.91 directory and running 'patch -u -p 0 < ../patch-127526-
>         2004.08.11'... I suppose you all know that though...
>         
>         Sorry if I have done something wrong in this note, this is my
>         first attempted contribution to any open source project
>         (except my own). If anyone has any suggestions or comments or
>         problems, please respond!
>         
>         Tim
>         
>         PATCH ---


> diff -upr ./mail/ChangeLog ../evolution-patch/mail/ChangeLog
> --- ./mail/ChangeLog 2004-07-19 07:51:56.000000000 -0500
> +++ ../evolution-patch/mail/ChangeLog 2004-08-11 07:48:34.000000000 -
> 0500
> @@ -1,3 +1,10 @@
> +2004-08-09  Tim Horton  <rhorton16 adelphia net>

there should be a blank line here to be the same format as other
changelogs :)

> + * mail-component.c (mail_component_set_default,
> + mail_component_ask_default, mail_component_init): Added a dialog
> + to ask the user if they want Evolution to be set as the default
> + mail client - this then sets the proper copy of Evolution to be
> + GNOME's mailto: handler. This should fix bug #127526.
> +
> 2004-07-13  Jeffrey Stedfast  <fejj novell com>
> 
> * em-folder-view.c (emfv_message_reply): Chck that the selection
> diff -upr ./mail/em-mailer-prefs.c ../evolution-patch/mail/em-mailer-
> prefs.c
> --- ./mail/em-mailer-prefs.c 2004-06-25 21:21:09.000000000 -0500
> +++ ../evolution-patch/mail/em-mailer-prefs.c 2004-08-11
> 18:31:59.000000000 -0500
> @@ -724,6 +724,12 @@ em_mailer_prefs_construct (EMMailerPrefs
> toggle_button_init (prefs, prefs->confirm_expunge, FALSE,
>     "/apps/evolution/mail/prompts/expunge",
>     G_CALLBACK (toggle_button_toggled));
> +     
> + /* Set as Default Mailer */
> + prefs->prompt_default_mailer = GTK_TOGGLE_BUTTON
> (glade_xml_get_widget (gui, "chkPromptDefaultMailer"));
> + toggle_button_init (prefs, prefs->prompt_default_mailer, FALSE,
> +     "/apps/evolution/mail/prompts/default_mailer",
> +    G_CALLBACK (toggle_button_toggled));

the folded lines should be aligned to one space past the '(' of the
previous line (emacs should auto-magically do this for you but I guess
you don't use emacs)

> /* New Mail Notification */
> locked = !gconf_client_key_is_writable (prefs->gconf,
> "/apps/evolution/mail/notify/type", NULL);
> diff -upr ./mail/em-mailer-prefs.h ../evolution-patch/mail/em-mailer-
> prefs.h
> --- ./mail/em-mailer-prefs.h 2004-05-19 10:07:15.000000000 -0500
> +++ ../evolution-patch/mail/em-mailer-prefs.h 2004-08-11
> 15:28:25.000000000 -0500
> @@ -83,6 +83,9 @@ struct _EMMailerPrefs {
> struct _GtkOptionMenu *empty_trash_days;
> struct _GtkToggleButton *confirm_expunge;
> 
> + /* Default Mailer */
> + struct _GtkToggleButton *prompt_default_mailer;
> + 

'default_mailer' would suffice, but that's just nitpicking... I really
don't care too much about this :)

> /* New Mail Notification */
> struct _GtkToggleButton *notify_not;
> struct _GtkToggleButton *notify_beep;
> diff -upr ./mail/evolution-mail.schemas.in.in ../evolution-
> patch/mail/evolution-mail.schemas.in.in
> --- ./mail/evolution-mail.schemas.in.in 2004-05-19 10:07:15.000000000
> -0500
> +++ ../evolution-patch/mail/evolution-mail.schemas.in.in 2004-08-11
> 18:32:54.000000000 -0500
> @@ -645,6 +645,21 @@
>           </long>
>        </locale>
>      </schema>
> +    
> +    <schema>
> +      <key>/schemas/apps/evolution/mail/prompts/default_mailer</key>
> +      <applyto>/apps/evolution/mail/prompts/default_mailer</applyto>
> +      <owner>evolution-mail</owner>
> +      <type>bool</type>
> +      <default>true</default>
> +      <locale name="C">
> +         <short>Prompt when Evolution isn't the default mail
> client</short>
> +         <long>
> +          Prompt when user starts Evolution and it's not the default
> +          mail client.
> +         </long>
> +      </locale>
> +    </schema>
> 
>      <!-- Trash settings -->
> 
> @@ -811,6 +826,5 @@
>           </long>
>        </locale>
>      </schema>
> -
>    </schemalist>
> </gconfschemafile>
> diff -upr ./mail/mail-component.c ../evolution-patch/mail/mail-
> component.c
> --- ./mail/mail-component.c 2004-06-25 21:21:09.000000000 -0500
> +++ ../evolution-patch/mail/mail-component.c 2004-08-11
> 18:39:13.000000000 -0500
> @@ -69,7 +69,13 @@
> 
> #include "e-task-bar.h"
> 
> +#include <gtk/gtk.h>

including this negates the need of the other #includes. however, the
reason we didn't #include <gtk/gtk.h> is to try to reduce compile times,
so preferably we wouldn't include this header unless absolutely
encessary. better to #include only the required headers.

> +#include <gtk/gtkdialog.h>
> #include <gtk/gtklabel.h>
> +#include <gtk/gtkwidget.h>
> +
> +#include <gconf/gconf-client.h>
> +#include <gconf/gconf.h>
> 
> #include <e-util/e-mktemp.h>
> 
> @@ -859,6 +865,72 @@ mail_component_init (MailComponent *comp
> 
> offline = mail_offline_handler_new();
> bonobo_object_add_interface((BonoboObject *)component, (BonoboObject
> *)offline);
> + 
> + mail_component_ask_default();
> +}
> +
> +void
> +mail_component_ask_default()

this should be:

static void
mc_prompt_default_mailer (void)

or something... definetely should be static tho, since this doesn't need
to be a public API.

> +{
> + /* Ask the user if they want Evolution to be their default mail
> client */

comment isn't really needed. better to just remove it.

> + 
> + GConfClient *gconf_client;

we normally just call this 'gconf' in the rest of the code. would be
nice if you could do the same here for consistancy.

> + GString *current_default_mailer;

shorter names would be good... perhaps 'current' ?

> + gboolean *current_no_repeat;

this should not be a pointer. also, perhaps a better name would be
'prompt' ?

always put an empty line after variable declarations to make the code
more readable.

> + gconf_init(0, NULL, NULL);

this shouldn't be necessary.

> + gconf_client = gconf_client_get_default();
> +
> + current_default_mailer = g_string_new(gconf_client_get_string
> (gconf_client,
> + "/desktop/gnome/url-handlers/mailto/command", NULL));

gconf_client_get_string() returns a malloc'd string pointer, so you are
leaking memory here.

also, I don't think 'current' needs to be a GString at all

> + current_no_repeat = gconf_client_get_bool(gconf_client,
> + "/apps/evolution/mail/prompts/default_mailer", NULL);
> + 
> + if(!(g_string_equal(current_default_mailer, g_string_new

space between if and (

> ("evolution-" BASE_VERSION " %s"))) &&
> + (current_no_repeat))

this is better done this way:

if (!strncmp (current, "evolution", 9) && prompt) {

(note: we don't want to check "evolution-1.5 %s" because the user may
add/change the arguments and we don't really want to mess with versions
I don't think, because stable versions will have a symlink from
evolution to evolution-$BASE_VERSION)

> + {
> + GtkWidget *default_mailer_dialog,
> *default_mailer_no_repeat_checkbox;

I think 'dialog' suffices as a variable name.

the other variable name should be shortened too (perhaps 'dont_ask'? or
maybe see what we called it in other code)

oh! actually. I think you should be using e_error_dialog's here so that
the dialog is HIG spaced and all that crap.

> + gint default_mailer_response;
> + gchar *default_mailer_message;

please use int and char rather than gint/gchar (it makes syntax
highlighting work in our emacs buffers)

> + 
> + default_mailer_dialog = gtk_message_dialog_new_with_markup(NULL,
> + GTK_DIALOG_MODAL, GTK_MESSAGE_QUESTION, GTK_BUTTONS_YES_NO, 
> + _("Would you like to make Evolution your default mail
> application?"));
> + 
> + gtk_dialog_set_default_response(GTK_DIALOG(default_mailer_dialog),
> GTK_RESPONSE_YES);
> + default_mailer_no_repeat_checkbox = gtk_check_button_new_with_label
> (_("Don't ask me again"));
> + gtk_container_add(GTK_CONTAINER(GTK_DIALOG(default_mailer_dialog)-
> >vbox), default_mailer_no_repeat_checkbox);
> + gtk_container_set_border_width(GTK_CONTAINER(GTK_DIALOG
> (default_mailer_dialog)->vbox), 12);
> + 
> + gtk_window_set_position(GTK_WINDOW(default_mailer_dialog),
> GTK_WIN_POS_CENTER);
> + gtk_widget_show_all(default_mailer_dialog);
> + default_mailer_response = gtk_dialog_run((GtkDialog
> *)default_mailer_dialog);
> + 
> + if(default_mailer_response == GTK_RESPONSE_YES)
> + mail_component_set_default();
> + 
> + if(((GtkToggleButton *)default_mailer_no_repeat_checkbox)->active)
> + gconf_client_set_bool(gconf_client,
> "/apps/evolution/mail/prompts/default_mailer", FALSE, NULL);
> + 
> + gtk_widget_destroy(default_mailer_dialog);
> + }
> + 
> + g_object_unref(gconf_client);
> +}
> +
> +void
> +mail_component_set_default()
> +{
> + GConfClient *gconf_client;

again, 'gconf'

> + gconf_init(0, NULL, NULL);

this is not needed.

> + gconf_client = gconf_client_get_default();
> + 
> + gconf_client_set_string(gconf_client, "/desktop/gnome/url-
> handlers/mailto/command", "evolution-" BASE_VERSION " %s", NULL);

this should arguably just be "evolution \"%s\""

I'm not really sure tho, perhaps JP can comment?

> + gconf_client_set_bool(gconf_client, "/desktop/gnome/url-
> handlers/mailto/enabled", TRUE, NULL);
> + gconf_client_set_bool(gconf_client, "/desktop/gnome/url-
> handlers/mailto/needs_terminal", FALSE, NULL);
> +
> + g_object_unref(gconf_client);
> }
> 
> /* Public API.  */
> diff -upr ./mail/mail-component.h ../evolution-patch/mail/mail-
> component.h
> --- ./mail/mail-component.h 2004-05-19 10:07:15.000000000 -0500
> +++ ../evolution-patch/mail/mail-component.h 2004-08-09
> 14:40:20.000000000 -0500
> @@ -96,4 +96,7 @@ struct _CamelStore *mail_component_peek_
> struct _CamelFolder *mail_component_get_folder(MailComponent *mc, enum
> _mail_component_folder_t id);
> const char *mail_component_get_folder_uri(MailComponent *mc, enum
> _mail_component_folder_t id);
> 
> +void mail_component_set_default();
> +void mail_component_ask_default();
> +

we don't really want these to be public API.

Jeff

-- 
Jeffrey Stedfast
Evolution Hacker - Novell, Inc.
fejj ximian com  - www.novell.com

Attachment: smime.p7s
Description: S/MIME cryptographic signature



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]