(In reply to comment #0)
> The low disk space warning implemented in bug 557647 lets you launch Baobab, if
> it is installed, to examine how the disk space is being used. This is fine and
> nifty.
> 
> The simplest solution to a problem of low disk space, however, is to empty the
> Trash. So if (and only if) the nearly-full partition has any items in the
> Trash, the warning should include emptying the Trash in its suggestions, and
> offer a button for doing that.
> 
> Mini-spec: <http://live.gnome.org/LowDiskSpaceWarning>
> 

I would love to work in this asap since i was asked to submit a patch for my GSoC application. I would be kind of you to help me through

What exactly do you need/expect? Matthew has summed it up pretty nicely on the wiki, the g-s-d plugin code is in svn under gnome-settings-daemon/trunk/plugins/housekeeping, and I'm sure you can take a peek at the baobab and the trash applet (gnome-panel) sources to find out how work with disks and the trash.

FWIW, I'm currently doing some work to implement Matthew's spec in Ubuntu, and the trash emptying will form part of that work.

Here's an initial patch that implements the spec at http://live.gnome.org/LowDiskSpaceWarning (including the trash emptying - borrowed from gnome-applets and adapted slightly).

I had to change the structure of the code a little. The old code just iterated through every GUnixMountEntry and displayed a notification if it was low on space. The spec dictates that you know in advance if there are any other usable volumes, and it also states "When a non-removable, non-hotpluggable partition is running low on disk space (how low?), an alert box should appear unfocused to warn you of this and advise you what to do".

To do this, the new code builds a list of static mounts using GUnixMountPoint. It then maps these in to a corresponding GUnixMountEntry if the GUnixMountPoint is mounted. I couldn't figure out any other way of doing this. The effect of this is that it ignores dynamically mounted media, which I think is what the spec intended.

It then iterates through the list of GUnixMountEntry's, checking the space available. Once this has happened for every mount, it displays the warning(s).

The dialog also has a checkutton to disable future warnings, as requested here: https://bugs.edge.launchpad.net/ubuntu/+bug/390504

I also made the thresholds configurable via GConf. Some of the comments already in the code suggested that was already planned, but I had to implement it to make the testing of it a bit easier.

As it's quite a big patch, I've also hosted the code on http://github.com/chrisccoulson/gnome-settings-daemon/tree/housekeeping_ldsm_notification

Created attachment 137490 [details] [review]
Patch to implement http://live.gnome.org/LowDiskSpaceWarning

Created attachment 137491 [details]
This is what the warning looks like (with trash)

Created attachment 137492 [details]
This is what the warning looks like (without trash)

While you're working on this, would you mind polishing it even further? Here are a few ideas:
- show the size of files in the Trash, and don't suggest emptying the Trash if it's empty. You could go even further and compute whether emptying the Trash will be sufficient to get above the threshold; if not, then removing files or programs will be required. This can be used to customize the message, e.g. if the Trash is gigantic, you can just tell the user to empty it, without speaking of removing files and programs at all.

- detect critical system mounts and give an explanation about the problems that may arise. If /, /var/, /home/ are full, the system can run into trouble quickly, while custom mounts in /mnt/ are not really an issue. Is it even useful to warn about non-system mounts being full?

- the icon for the "Ignore" button is a little aggressive for what it does. That doesn't seem really consistent with other uses of such icons in the desktop. I'd rather use something like gtk-close or gtk-quit.


But overall I really like that feature!

Thank you for your comments. I've written a quick response each of them in turn:

1) That would be quite interesting to implement. However, it could create a lot of up-front disk IO, as I'd need to stat() every file in the trash before displaying the dialog. That could take some time if a user doesn't empty their trash very often (think about how long it takes for Nautilus to display folder size when you open the properties window on a folder with lots of files). The current implementation will only display the "Empty Trash" button if there are files in the users trash folder for that particular mount.

2) I think that the only issue with this point would be "information overload". There is already quite a lot of text in the dialog, and adding more might make it confusing.

3) The icon for the ignore button is just a GTK_STOCK_CANCEL. i think it looks aggressive on my system due to the icon theme I use.

Looks like a high quality patch in general.

* This is totally minor, but i'd say "Free percentage notify threshold" instead of "Free percent".

+                *ignore_paths = g_slist_remove (*ignore_paths, found->data);

Are we leaking the mount point here?  We strdup it in the update case, but remember g_slist_remove won't g_free the data for you.

I glanced over the rest of the patch and didn't see any obvious superficial code issues, but I don't know the current code.

One thing I'm wondering - do you determine if hte user can write to the mounts?  

In general I think one of the gnome-settings-daemon maintainers should apply this patch.

Thanks Colin,

I think that mount point does get leaked. I'll fix that when I get the chance. I did run it through valgrind when I wrote the patch, but I probably haven't tested every possible combination yet.

The patch currently does not check if the user can write to the mount - I wasn't sure of any non-hacky way to do that. In any case, if the user can't write to the mount, the mount shouldn't have any of their trash on it, so the only options displayed will be "Ignore" and "Examine".

Some additional comments:

-#include "config.h"

config.h must always be the first include. Don't remove this.

+                if (!g_file_test (trash_files_dir, G_FILE_TEST_EXISTS) || !g_file_test (trash_files_dir, G_FILE_TEST_IS_DIR)) {

Checking for just _IS_DIR is sufficient. It won't be if it doesn't exist. ;-)

+                        trash_dir = g_strdup_printf (".Trash-%s", uid);
+                        trash_files_dir = g_build_filename (path, trash_dir, "files", NULL);
+                        if (!g_file_test (trash_files_dir, G_FILE_TEST_EXISTS) || !g_file_test (trash_files_dir, G_FILE_TEST_IS_DIR)) {
+                                g_free (trash_files_dir);
+                                g_free (trash_dir);
+                                g_free (uid);
+                                return has_trash;
+                        }
+                        g_free (trash_dir);

If you free trash_dir right after using it to create trash_files_dir you'll 
save an instruction in the branch.

+        if (dialog->priv->other_partitions)
+                text = g_strdup (_("Don't show any warnings again for this filesystem"));
+        else
+                text = g_strdup (_("Don't show any warnings again"));

Since those texts are static it looks like you don't need to dup them.
Same with gsd_ldsm_dialog_get_secondary_text.

Created attachment 138688 [details] [review]
Patch to implement http://live.gnome.org/LowDiskSpaceWarning

Thanks Jens / Colin,

Here is an updated version of the patch.

Created attachment 138689 [details]
Diff between patches

And here are the changes I made between the first version and the newer one.

Thanks, Chris. Feel free to commit to trunk.

Since we're already in the string/UI announcement period it would be great if you could send the respective mails out (cf. http://live.gnome.org/ReleasePlanning/Freezes).

Already applied some time ago it seems. Somebody forgot to close this.

That will be me - I'm not allowed to change the status