GNOME Bugzilla – Bug 138930
nautilus has no option to not show Volumes on the desktop
Last modified: 2006-03-21 23:26:11 UTC
There are options to show Home, Computer and Trash, but not Volumes. For those of us who like really empty desktops. I think this it's only fair.
Created attachment 26277 [details] [review] preliminary patch This is the preliminary patch, it likes to break (or even do nothing) when you call desktop_volumes_visible_changed() but it's a start. It will work if you start nautilus with the setting set (I know that's not the gconf way).
Created attachment 26307 [details] [review] an improved patch This patch seems to basically work, however it will SEGV if you toggle the gconf key too much (I think I'm forgetting to clean something up). It also seems to SEGV if you make volumes visible, open a folder for the volume, make volumes hidden and unmount the volume, I suspect I'm not quite dealing with the possibility of the list containing null-references.
*** Bug 139056 has been marked as a duplicate of this bug. ***
Confirming & Adding PATCH keyword. Davyd, it would be best to send the patch to nautilus-list too - when you feel confident enough about it. A small thing I noticed: "NautilusDesktopLink *link = NULL;" violates the Nautilus Style Guide ("We do not initialize local variables in their declarations.")
I mailed the nautilus-list, however I am waiting for moderation. If you don't initialize with the declaration, is it ok to initilize a little further down? ie: NautilusDesktopLink *link; SomeType *somepointer; ... *link = NULL; After all, if the variable isn't initialized, it will throw a warning and stop the compile.
This patch introduces a new string, and we're still in string freeze for HEAD. I don't want to branch for 2.8 yet, since there are still a few important bugs we have to fix. Comment on the patch: + + GnomeVFSVolumeMonitor *volume_monitor; No need to store this. This is a global object. + NautilusDesktopLink *link = NULL; + No initialization in declaration accordint to the coding style. + if (l && l->data) { The items in the list should always be non-NULL. Why is this added? + /* + * this method allows us to dynamically show and unshow the + * mounted volumes on the desktop. + */ Function comments before the function, not inside it. Although this comment doesn't look that useful overall. + NautilusDesktopLinkMonitor *monitor; + monitor = NAUTILUS_DESKTOP_LINK_MONITOR (callback_data); + + GList *l, *volumes; No code before declarations in ansi C. + if (eel_preferences_get_boolean (NAUTILUS_PREFERENCES_DESKTOP_VOLUMES_VISIBLE)) { We might have been called before with the same value. At least check (monitor->details->volume_links == NULL) before creating new ones. + g_list_foreach (monitor->details->volume_links, (GFunc)g_object_unref, NULL); Leaves the list with dangling pointers. You need to free the list and set volume_links to NULL. + /* + * added to allow people to dynamically show and unshow volumes + */ Comments like these aren't really needed. The reason for the next line is obvious from the code itself. We can't have a comment in from of each line stating what the line does. Comments are useful when the code does something complicated, unusual or mentions something that the code doesn't say.
Created attachment 26435 [details] [review] even more improved patch This addresses the issues of the previous patch (as raised by Alex and others). I have fixed the coding style mistakes. Removed one or two weird lines of code and the comments (which I should have removed, they were just my placeholders). Have fixed the two bugs I knew about. Should also now be freeing things correctly. Are you sure that volume_monitor is global? Perhaps I did something stupid...
what he means is that gnome_vfs_get_volume_monitor() returns a singleton object, so you can just call that func when you need the monitor, rather than storing a pointer to it.
*** Bug 140571 has been marked as a duplicate of this bug. ***
Created attachment 27416 [details] [review] further improvements to previous patch Ok, no more storing volume_manager in the struct. Also fixed a grammatical mistake I noticed in the schema. I don't think I've done anything else strange.
*** Bug 62736 has been marked as a duplicate of this bug. ***
Commited to HEAD.
*** Bug 74636 has been marked as a duplicate of this bug. ***
*** Bug 143378 has been marked as a duplicate of this bug. ***
Perhaps these options should be made user-visible? Now that the Places menu and the mount applet are so widely user, maybe that'd be a good idea!