Re: [patch] size field for multi window property dialogs

[Dave Camp <dave ximian com>]

On Mon, 2003-06-30 at 17:31, Johan Dahlin wrote:
> Attached patch that adds file size to multi window property dialogs. The
> patch is slightly larger than it should be, because I fixed so it fits
> in a 80 column terminal in a few places.
> 
> Please put me on cc for all replies, since I'm not subscribed.
> 
> The patch is against CVS HEAD.


>  static NautilusFile *
>  get_target_file (FMPropertiesWindow *window)
>  {
> -       g_return_val_if_fail (!is_multi_file_window (window), NULL);
> -
>         return NAUTILUS_FILE (window->details->target_files->data);
>  }

The idea with this assertion is that anything "safe" for multi-file
windows shouldn't be using this function, so this should probably stay.


> +               for (l = window->details->original_files; l; l =
> l->next) {
> +                       file = NAUTILUS_FILE (l->data);

You should be iterating over target_files here.

> +                       status = nautilus_file_get_deep_counts (file, 
> +                                                               &dir_count,
> +                                                               &file_count, 
> +                                                               &unreadable,
> +                                                               &file_size);
> +                       if (nautilus_file_is_directory(file)) {
> +                               total_count += (file_count +
> dir_count);
> +                               total_size += file_size;
> +                       } else {
> +                               total_count++;
> +                               total_size +=
> nautilus_file_get_size(file);
> +                       }

The get_deep_counts() call should probably be inside the if
(nautilus_file_is_directory()) {} block (same with the unreadable).

> if (is_multi_file_window (window)) {
...
> +       } else {
> +               file = get_target_file (window);
> +               g_assert (nautilus_file_is_directory (file) ||
> +                         nautilus_file_is_gone (file));
> +               
> +               status = nautilus_file_get_deep_counts (file, 
> +                                                       &dir_count, 
> +                                                       &file_count, 
> +                                                       &unreadable_dir_count,
> +                                                       &total_size);
> 

Does this need to be a separate clause?  It seems like running through
the multi-file case with only one element should work fine.

> +       if (is_multi_file_window (window)) {
> +               GList *l;
> +               for (l = window->details->original_files; l; l =
> l->next) {
> +                       file = NAUTILUS_FILE (l->data);
> +                       nautilus_file_recompute_deep_counts (file);
> +                       
> +                       g_signal_connect_object (file,
> +                              "updated_deep_count_in_progress",
> +                               G_CALLBACK
> (schedule_directory_contents_update),
> +                              window, G_CONNECT_SWAPPED);
> +               }
> +       } else {
> +               file = get_target_file (window);
> +               nautilus_file_recompute_deep_counts (file);
> +               
> +               /* Connect to signal to update value when file
> changes. */
> +               g_signal_connect_object (file,
> +                      "updated_deep_count_in_progress",
> +                      G_CALLBACK
> (schedule_directory_contents_update),
> +                      window, G_CONNECT_SWAPPED);

This also probably doesn't need a seperate multi-file clause.

>         if (is_multi_file_window (window)) {
> -               should_show_count = FALSE;
> +               GList *l;
> +               guint count = 0;
> +               NautilusFile *file;
> +               
> +               for (l = window->details->original_files; l; l =
> l->next) {
> +                       file = NAUTILUS_FILE (l->data);
> +                       count +=
> nautilus_file_should_show_directory_item_count (file);
> +               }
> +               should_show_count = count;
>         } else {
>                 should_show_count =
> nautilus_file_should_show_directory_item_count (get_target_file
> (window));
>         }

Here too.

Also, there seems to be a problem with setting deep_count_finished.  It
sets deep_count_finished if status == NAUTILUS_REQUEST_DONE, but status
is always the status of the last file in the list.  If the last file in
the list finishes before the others, you set deep_count_finished too
early.

-dave