Re: [PATCH] Add tree functionality to list view

[Alexander Larsson <alexl redhat com>]

On Tue, 2005-06-14 at 18:38 +0200, Jürg Billeter wrote:
> On Die, 2005-06-14 at 18:04 +0200, Alexander Larsson wrote:
> > Here are some more detailed review comments:
> 
> Thanks for your quick response.

I commited your second patch as a first step to get this in.
There are still some issues i'd like to see solved:

* Duplicate with two files in different subdirs selected doesn't work.

* Copy/Paste and DnD semantics with both a file and its parent folder
selected isn't right.
I'll take a look at this.

* Maybe we can fix the flashing from Loading... to (Empty) and then to a
filename by some clever timeout?

* We should be releasing subdirectories for collapsed subdirectories for
performance/resource reasons. At least after a while. 

* The selection behaves a bit strange wrt to the dummy (Empty) row. I
can't select it by itself, but i can select it with a range select. And
if i double click on it when something else is selected i open the
selection.


> >  	/* Let the world know about our new order */
> > -	path = gtk_tree_path_new ();
> >  
> >  	g_assert (new_order != NULL);
> >  	gtk_tree_model_rows_reordered (GTK_TREE_MODEL (model),
> >  				       path, NULL, new_order);
> >  
> > -	gtk_tree_path_free (path);
> > 
> > This calls gtk_tree_model_rows_reordered multiple times when we recurse.
> > Should only call it once in the end.
> 
> The way I understand the documentation[1], it's not possible to emit row
> order changes in multiple subtrees at once or did I miss something?

Nah, I missed something.

> >  static void
> >  fm_list_model_remove (FMListModel *model, GtkTreeIter *iter)
> > 
> > +			if (child_file_entry->file != NULL) {
> > +				fm_list_model_remove_file (model, child_file_entry->file);
> > +			} else {
> > +				g_sequence_remove (child_ptr);
> > +			}
> > 
> > Don't you need to call gtk_tree_model_row_deleted for the dummy node?
> > And update the stamp.
> 
> Yes, but as I look at it, it may be cleaner if I just iterate over
> GtkTreeIter objects and call fm_list_model_remove for all of them,
> including dummy nodes, right?

I'm not sure what you mean here.

> Yeah, is probably more what people would expect, although it'd feel
> somehow better if there was no need for dummy nodes at all, i.e. better
> gtk+ support for partial loaded tree models.

Yeah. Now that kris is working on the tree view again, maybe we can get
him to look into this.

> >  fm_list_view_add_file (FMDirectoryView *view, NautilusFile *file)
> >  {
> > -	fm_list_model_add_file (FM_LIST_VIEW (view)->details->model, file);
> > +	FMListModel *model;
> > +	
> > +	model = FM_LIST_VIEW (view)->details->model;
> > +	if (!fm_list_model_add_file (model, file)) {
> > +		fm_directory_view_remove_sub_directory (view,
> > +					nautilus_directory_get_for_file (file));
> > +		fm_list_model_remove_file (model, file);
> > +		fm_list_model_add_file (model, file);
> > +	}
> >  }
> > 
> > A better approach would be to remove subdirectories on row_collapsed.
> > That way we don't use resources for expanded and later collapsed subdirs.
> 
> Wouldn't be that easy AFAICT as there might be a problem when dragging
> an expanded directory into another expanded directory. That's the reason
> I've added the remove code to add_file. Although it may make sense to
> additionally remove subdirectories on row_collapsed, too, to release
> resources. Or might the additional delay when collapsing and
> re-expanding a large subdirectory be too big?

Can you explain this in more detail? I don't really understand the
issue.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's an underprivileged coffee-fuelled matador plagued by the memory of his 
family's brutal murder. She's a sharp-shooting antique-collecting queen of the 
dead from a family of eight older brothers. They fight crime!