Re: [Nautilus-list] Re: [Updated Patch] Use GConf for background settings

[Håvard Wigtil <havardw stud ntnu no>]

On Sun, 2002-02-10 at 01:02, Darin Adler wrote:

> > I don't think that the "Reset Desktop Background" option in the desktop
> > menu makes much sense for Gnome2, as MetaTheme will handle desktop
> > themes. I think it should be removed (and I'll do it together with the
> > code removal patch I promised if you agree), but it could also be made
> > to reset according to the current metatheme. But that sould be a job for
> > the metatheme capplet IMHO.
> 
> I don't agree. No matter what part of the system does the theming, I think
> that a choice in the right click menu to restore the desktop to the theme
> default is a nice thing to have. I do not really care how we make it work,
> but I think it should continue to work.

I'll take a look at what Metatheme does with backgrounds.


> > I get a warning if I don't, probably because desktop_theme_source is
> > defined as static.
> 
> I don't think that "static" is the issue, but perhaps "const" is.

Sorry, I meant "const".

> 
> This code is crying out for a version of gconf_value_get_string that handles
> the NULL and the type-checking and returns NULL if something is wrong.
> Because otherwise we have the same sequence of if statements in every
> function. Cut and paste code bad -- shared functions good.

I agree, I thought about it when I wrote the code. How about adding 
eel_gconf_value_get_xxx (GConfValue) and
eel_gconf_value_get_xxx_with_default (GConfValue, default) to
eel-gconf-extensions? I could do this while working over the patch
again.

> > +        primary_color = gconf_value_get_string (entry->value);
> 
> So primary_color can't be NULL here? Is it guaranteed that the string is
> non-NULL if the type is GCONF_VALUE_STRING.

If I understand GConf correctly it isn't possible to set something to a
NULL value. And that the patch survives "gconf --break-key" seems to
agree with this. 

> I would have liked to see a bit more shared code. It does bother me that
> nautilus_desktop_set_background has code that duplicates decisions made in
> each of the four callbacks; that code should be shared and not copied and
> pasted.

I'll go over the patch again and see what I can do. I don't have much
time today or tomorrow, so you'll probably get it tuesday.

-- 
   Håvard

mailto:havardw stud ntnu no||http://www.stud.ntnu.no/~havardw||73 52 55
76
All it takes to start an avalanche is a single snowflake||Or a
snowboarder
        Oh! Un Fraggle! Regarde, maman! J'ai attrapé un Fraggle!