On Hën , 2003-11-24 at 19:49, David Moore wrote: > Thanks for the feedback. I've attached an updated patch which hopefully addresses all the issues. > > > perhaps it should just do 'set path "", set path filename', rather than having to check it first? > > setting the path to "" causes GNOME to remove the current image and draw a gradient before drawing the next image. That's less efficient than simply drawing the next image, so I figured I might as well avoid it when possible. What you want to do is, if the full string for the gconf key is the same as what you are aout to set it to, do nothing, otherwise, set it to the new image filename. > > should we explictly set stretched, or just let it be done via the control panel? > > I set "stretched" so that the image would appear when picture_options had been set to "none" (which is the default in several distros), because otherwise, the image would be there but GNOME wouldn't display it. I've changed it now so that it sets picture_options to "stretched" if "none" had been set, but otherwise leaves it alone. You want to default to "wallpaper", not "stretched". This prevents very small images from being stretched up to a very large screen. Another good default might be "centered", since it avoids the stretching issue as well. > > there's some inconsistency above, in one case it pops up an error, in another it doesn't, you're testing basically the same thing twice too (i.e. if it exists and is a normal file, then if it exists and can be written to). > > I've added an error message to the other test, so it's consistant. I'm leaving both tests in place because it's possible (if .background already exists as a directory, unlikely as that may be) for the 'stat' test to fail while the 'access'test doesn't. That particular bit of code was copied from emu_can_save(), so the same problem also exists there. I didn't change it, but someone might want to. We shouldn't just store the image as "~/.background/" really. It adds yet another dot-file to deal with. If we had an implementation for the XDG Spec to use, I would suggest using the more appropriate path of "$XDG_DATA_HOME/Wallpapers/$filename", but maybe we can just use a path of "~/.gnome2/Wallpapers/$filename" for now instead. -- dobey
Attachment:
signature.asc
Description: This is a digitally signed message part