Re: [PATCH] file size and device in properties dialog

[Alexander Larsson <alexl redhat com>]

On Wed, 19 Mar 2003, Gaute Lindkvist wrote:

> This patch gives Nautilus file size and device path in the properties
> dialog, like so:
> http://www.stud.ntnu.no/~lindkvis/patch.jpg
> 
> I'm not an experienced C-coder, and I've tried to be careful about freeing
> strings, but I probably still leak memory :)
> 
> I do think the conceptual part of the patch is right though. I've tested
> it with lots of URIs (including trash, nfs-mounts, ssh and smb) and it
> seems to handle exceptions gracefully  (shows unknown, instead of
> crashing).
> 
> The patch has been uploaded to bugzilla:
> http://bugzilla.gnome.org/show_bug.cgi?id=48144
> 
> It does use a couple of extra strings and really is a feature enhancement,
> so not good for 2.2.x anyway. But 2.4 branch is perhaps open?

Hmmm. I'm not sure showing the device name in a highly visible dialog like 
that is very good. However, showing free space for directories sounds like 
a good thing.

We've not branched yet, but the 2.2.x branch is slightly thawed, so some 
small important string changes can go in. (We have to announce them 
though.)

Comments on the patch:
Index: libnautilus-private/nautilus-file.c
===================================================================
RCS file: /cvs/gnome/nautilus/libnautilus-private/nautilus-file.c,v
retrieving revision 1.314
diff -u -r1.314 nautilus-file.c
--- libnautilus-private/nautilus-file.c	18 Mar 2003 18:12:14 -0000	1.314
+++ libnautilus-private/nautilus-file.c	18 Mar 2003 23:46:36 -0000
@@ -4565,6 +4571,63 @@
 
 	/* Non-broken symbolic links return the target's type for get_file_type. */
 	return nautilus_file_get_file_type (file) == GNOME_VFS_FILE_TYPE_SYMBOLIC_LINK;
+}
+
+/**
+ * nautilus_file_get_device_free_space
+ * Get a nicely formatted char with free space on the file's device
+ * @file: NautilusFile representing the file in question.
+ *
+ * Returns: newly-allocated copy of file size in a formatted string
+ */
+char *
+nautilus_file_get_device_free_space (NautilusFile *file)
+{
+	char * file_uri;
+	char * local_path;
+	GnomeVFSFileSize free_space;
+	GnomeVFSResult result;
+
+	file_uri = nautilus_file_get_uri (file);
+	if (file_uri == NULL) {
+		return NULL;
+	}
+	local_path = gnome_vfs_get_local_path_from_uri (file_uri);
+	g_free (file_uri);
+	if (local_path == NULL) {
+		return NULL;
+	}
+	result = gnome_vfs_get_volume_free_space (gnome_vfs_uri_new(local_path), &free_space);

gnome_vfs_get_volume_free_space() takes a uri, not a path. Don't convert 
it using gnome_vfs_get_local_path_from_uri.

You leak the GnomeVFSURI object.

Nautils coding standards require a space before the paranthesis in "gnome_vfs_uri_new(local_path)"

+	g_free (local_path);
+	if (result == GNOME_VFS_OK) {
+		return g_strdup(gnome_vfs_format_file_size_for_display (free_space));

No need to strdup here. gnome_vfs_format_file_size_for_display() returns a 
copy already. 

Space missing.

+	}
+	else {

} and else go on the same line.

+		return NULL;
+	}
+}
+/**
+ * nautilus_file_get_device_path
+ * Get the path of the device the file resides on
+ * @file: NautilusFile representing the file in question.
+ * 
+ * Returns: newly-allocated copy of the device path of the target file
+ */ 
+char *
+nautilus_file_get_device_path (NautilusFile *file)
+{
+	char *file_uri;
+	
+	NautilusVolume *volume;
+	file_uri = gnome_vfs_get_local_path_from_uri (nautilus_file_get_uri (file));

You leak the uri from nautilus_file_get_uri.

The name file_uri is not good for a path (its not a uri).

+	volume = nautilus_volume_monitor_get_volume_for_path (nautilus_volume_monitor_get (), file_uri);
+	g_free (file_uri);
+	if (volume != NULL) {
+		return g_strdup (nautilus_volume_get_device_path (volume));
+	}
+	else {

} and else on the same line.

+		return NULL;
+	}
 }
 
 /**
Index: libnautilus-private/nautilus-file.h
===================================================================
RCS file: /cvs/gnome/nautilus/libnautilus-private/nautilus-file.h,v
retrieving revision 1.95
diff -u -r1.95 nautilus-file.h
--- libnautilus-private/nautilus-file.h	18 Mar 2003 18:12:14 -0000	1.95
+++ libnautilus-private/nautilus-file.h	18 Mar 2003 23:46:38 -0000
@@ -136,6 +136,8 @@
 gboolean                nautilus_file_is_mime_type                      (NautilusFile                   *file,
 									 const char                     *mime_type);
 gboolean                nautilus_file_is_symbolic_link                  (NautilusFile                   *file);
+char *                  nautilus_file_get_device_free_space             (NautilusFile                   *file);
+char *                  nautilus_file_get_device_path                   (NautilusFile                   *file);
 char *                  nautilus_file_get_symbolic_link_target_path     (NautilusFile                   *file);
 char *                  nautilus_file_get_symbolic_link_target_uri      (NautilusFile                   *file);
 gboolean                nautilus_file_is_broken_symbolic_link           (NautilusFile                   *file);
Index: src/file-manager/fm-properties-window.c
===================================================================
RCS file: /cvs/gnome/nautilus/src/file-manager/fm-properties-window.c,v
retrieving revision 1.167
diff -u -r1.167 fm-properties-window.c
--- src/file-manager/fm-properties-window.c	12 Mar 2003 14:41:42 -0000	1.167
+++ src/file-manager/fm-properties-window.c	18 Mar 2003 23:46:45 -0000
@@ -1593,10 +1593,10 @@
 	GtkWidget *container;
 	GtkWidget *icon_pixmap_widget, *icon_aligner, *name_field;
 	GtkWidget *button_box, *temp_button;
+
 	char *image_uri;
 	NautilusFile *target_file, *original_file;
 	GtkWidget *hbox, *name_label;
-
 	target_file = window->details->target_file;
 	original_file = window->details->original_file;
 

Please try to avoid unnecessary whitespace changes.

@@ -1694,6 +1694,12 @@
 		append_title_value_pair (table, _("Size:"), target_file, "size");
 	}
 	append_title_and_ellipsizing_value (table, _("Location:"), target_file, "where");
+	
+	append_title_value_pair (table, _("Device:"), target_file, "device");
+	
+	if (!should_show_link_target (window) && nautilus_file_is_local (target_file)) {
+		append_title_value_pair (table, _("Free space:"), target_file, "free_space");
+	}

Why do you use !should_show_link_target here? It seems quite unrelated. It 
should be just (should_show_free_space (window)), and then 
should_show_free_space has to implement whatever we thing is right for 
when to show the free space. Maybe only for local directories?

 	if (should_show_link_target (window)) {
 		append_title_and_ellipsizing_value (table, _("Link target:"), target_file, "link_target");
 	}

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a one-legged devious vampire hunter from the Mississippi delta. She's a 
cosmopolitan winged queen of the dead who don't take no shit from nobody. They 
fight crime!