[evolution-patches] Re: Bug #127526

[Jeffrey Stedfast <fejj ximian com>]

On Mon, 2004-01-12 at 10:39, Andres de Barbara wrote:
> El dom, 11-01-2004 a las 22:16, Jeffrey Stedfast escribió:
> > there doesn't seem to be any such bug report in bugzilla.ximian.com and
> > there is no attachment attached to your message - can you try
> > re-sending?
> 
> Ok this is the bug (bounty)
> 
> http://bugzilla.gnome.org/show_bug.cgi?id=127526

ah, didn't think to look on gnome bugzilla.

> 
> > also, please send to evolution-patches ximian com next time (so other
> > devs can look over the patch)
> 
> Look I'm really new on this and I don't want to be flame just because
> the name of my pointers are not ok. I'm emailing you, because you seen
> to be the contac (http://www.gnome.org/bounties/Mailer.php3#127526)
> I known you are busy but I just looking for guiance on the patch. 

ok, I was just mentioning that evolution-patches ximian com is a better
place to send it as it means other devs can also review it in case I'm
too busy or something :-)

anyways, I've added evolution-patches ximian com to the Cc list.

> 
> 
> > Jeff
> > 
> > On Sun, 2004-01-11 at 11:25, Andrés de Barbará wrote:
> > > Jeffrey:
> > >         I been working in a patch for this bug and I got some technicals
> > > issues(no really so technical). My current patch look like the
> > > attachment. Theidea begind is:
> > >       Evolution default mailer question will only if is the first time you
> > > arusing it or if you choosed it but other appl. has become the
> > > defaultmailer (only one time).
> > > 
> > > I really need opinions about it.
> > > thx Andres de Barbara
> > > 

for future reference, use `diff -u` and also include ChangeLog entries -
it helps us out a lot. Also, when attaching a patch in Evolution, in the
file-chooser dialog there is a checkbox called "Suggest show inline", it
would also help if when you send patches you could enable that as well.
Thanks...

more comments below:

Index: mail-component.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/mail-component.c,v
retrieving revision 1.40
diff -c -r1.40 mail-component.c
*** mail-component.c    9 Jan 2004 06:52:41 -0000       1.40
--- mail-component.c    11 Jan 2004 06:33:39 -0000
***************
*** 71,76 ****
--- 71,82 ----
  
  #define d(x) x
  
+ enum {
+         EVOLUTION_FIRST_TIME_QUESTION,
+         EVOLUTION_IS_NOT_DEFAULT_MAILER,
+         EVOLUTION_IS_DEFAUL_MAILER

misspelled this enum I think :-)

+ };
+ 
  
  #define PARENT_TYPE bonobo_object_get_type ()
  static BonoboObjectClass *parent_class = NULL;
***************
*** 505,516 ****
--- 511,592 ----
        /* mail_autoreceive_setup (); EPFIXME keep it off for testing */
        
        setup_search_context (component);
+ 
+       ask_evolution_default_mailer ();
        
        /* EPFIXME not sure about this.  */
        go_online (component);
  }
  
+ void 
+ ask_evolution_default_mailer (void)
+ {
+       
+       GtkWidget * dialog;
+       GConfClient * client;
+       gchar *default_mailer, *msg = NULL;
+       gint evolution_default_mailer;

we evolution mail hackers prefer to use char and int rather than
gchar/gint. also, I think you mean to use evolution_dedault_mailer as a
gboolean, right?

+       
+       client = gconf_client_get_default ();
+ 
+       default_mailer = gconf_client_set_string (client,
+                                                
"/desktop/gnome/url-handlers/mailto/command",
+                                                 NULL);
+       evolution_default_mailer = g_conf_client_get_int (client, 
+                                                        
"/apps/evolution/mail/promts/default_mailer",
+                                                         NULL);

the gconf key path here is misspelled also.

+       
+       if (!default_mailer || !strcmp("evolution
\"%s\"",default_mailer)) {

add a space between strcmp and the (

also add a space after the comma

I would also change the strcmp to:

strncmp (default_mailer, "evolution", 9) != 0

reason is because the user may change the arguments... also, your usage
of !strcmp() is wrong because that means they *do* match, not that they
don't match which is what you are using it to mean.

+               if (evolution_default_mailer ==
EVOLUTION_IS_NOT_DEFAULT_MAILER) {

shouldn't this value be gotten as a result of the strcmp rather than as
another gconf key which may or may not necessarily be in sync with each
other?

+                       g_object_unref (client);
+                       return;
+               }
+                               
+               dialog = gtk_message_dialog_new (NULL,
+                                               GTK_DIALOG_DESTROY_WITH_PARENT,
+                                               GTK_MESSAGE_QUESTION,
+                                               GTK_BUTTONS_YES_NO,
+                                               (evolution_default_mailer == EVOLUTION_FIRST_TIME_QUESTION) ?
+                                               _("Would you like to
make Evolution your default mail aplication?") :

s/aplication/application/g

+                                               _(msg = g_strdup_printf
("The default mailer is %s. "
+                                                                       
"Would you like to make Evolution "
+                                                                       
"your default mail aplication?",
+                                                                       
!default_mailer ? <unset> : default_mailer)));
+               

don't do the g_strdup_printf in the message_dialog_new call, do it
before.

also, what is <unset> here? I don't think that's valid c...

+                       if (msg == NULL)
+                               g_free (msg);
+ 
+               gtk_dialog_set_default_response (GTK_DIALOG (dialog), 
+                                                GTK_RESPONSE_NO);
+               
+               if (gtk_dialog_run (GTK_DIALOG (dialog)) ==
GTK_RESPONSE_YES) {
+                       gconf_client_set_string (client, 
+                                               "/desktop/gnome/url-handlers/mailto/command",
+                                               "evolution \"%s\"",
NULL);
+                       gconf_client_set_bool (client,
+                                             
"/desktop/gnome/url-handlers/mailto/enable",
+                                              TRUE, NULL);
+                       gconf_client_set_int (client,
+                                            
"/apps/evolution/mail/promts/default_mailer",
+                                            
EVOLUTION_IS_DEFAUL_MAILER, NULL);
+               } else {
+                       gconf_client_set_int (client,
+                                              
"/apps/evolution/mail/promts/default_mailer",
+                                              
EVOLUTION_IS_NOT_DEFAULT_MAILER, NULL);
+               } else if (evolution_default_mailer !=
EVOLUTION_IS_DEFAUL_MAILER)
+                       gconf_client_set_int (client,
+                                            
"/apps/evolution/mail/promts/default_mailer",
+                                            
EVOLUTION_IS_DEFAUL_MAILER, NULL);
+       
+               if (default_mailer == NULL)
+                       g_free (default_mailer);
  
+               gtk_widget_destroy (dialog);
+       }

I'm not sure your logic here makes sense...?

+  
+       g_object_unref (client);
+ }
+  
  /* Public API.  */
  
  MailComponent *



Jeff

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