Re: [evolution-patches] Ask user to make evolution default mail app bug #127526 (Mail)

[Not Zed <notzed ximian com>]

On Thu, 2004-08-19 at 13:22 +0300, Niklas Nylund wrote:
> > 
> > 
> > don't call gconf_init here.  it should already be initialised by here 
> > surely?  left over from prototype i guess?
> > 
>
> Makes sense, removed it.
>
> >> +       client = gconf_client_get_default ();
> >> +
> >> +       val = gconf_client_get (client, 
> >> "/apps/evolution/mail/prompts/default_mailer", NULL);
> >> +       if (val)
> >> +               prompt = gconf_value_get_bool (val);
> >> +       else
> >> +               prompt = TRUE;
> > 
> > 
> > I would just use gconf_client_get_bool.  Let the schema provide the 
> > default; this also lets administrators override the default.
> >
>
> Using gconf_client_get_bool isn't such a good idea, because it returns 
> FALSE if the key is unset (atleast I think it does). And it will be 
> unset the first time we launch evolution, so we got no idea if we should 
> show the dialog or not.

If you get false, just don't show it.  The default wont be false unless something has gone awry with the install, the default comes from the schema.

> >> +       val = gconf_client_get (client, 
> >> "/desktop/gnome/url-handlers/mailto", NULL);
> >> +       if (val) {
> >> +               mailapp = (gchar *) gconf_value_get_string(val);
> >> +               if (strcmp (mailapp, "evolution") == 0)
> >> +                       prompt = FALSE;
> >> +               g_free (mailapp);
> >> +       } else
> >> +               prompt = TRUE;
> > 
> > again i would just use gconf_client_get_string here.
> > If it is NULL or if it isn't evolution, then prompt == true
>
> I can't see a difference, that's exactly what I do, if the key is unset 
> prompt = TRUE, if not we check if it's evolution. I can't set prompt
> to true if mailapp == evolution since default_mailer could be set to false.
The difference is fewer lines of code and simpler to read.

If default_mailer is false you've already given up, or should have.
> And btw, the gchar * typecast is there because I couldn't figure out how
> to add glib.h properly.
I don't understand here, you're already including glib.h otherwise gchar wouldn't exist.

> >> +
> >> +       if (prompt == TRUE)
> >> +               if (em_utils_prompt_user (NULL, 
> >> "/apps/evolution/mail/prompts/default_mailer",
> >> +                                        
> >> "mail:check-default-mail-app", NULL))
> > 
> > 
> > you can combine this if into a single _expression_, makes it a bit more 
> > readable to me (less indenting).
> > 
> > if (prompt
> >    && em_utils_prompt_user( ...))
> > 
> done
>
> Btw, what's the proper way to make patches?
> When I use cvs diff -up, I get a bunch of unnecessary lines in the 
> beginning, like this:

Thats correct.  Just ignore them, patch will.

--

Michael Zucchi <notzed ximian com> "born to die, live to work, it's all downhill from here" Novell's Evolution and Free Software Developer