Re: timeout patch for properties - take 2

[Mathieu Lacage <mathieu_lacage myrealbox com>]

On Wed, 2003-09-17 at 16:55, Alexander Larsson wrote:
> On Mon, 2003-09-15 at 11:42, Mathieu Lacage wrote:
> > hi, 
> > 
> > here is the patch
> > 
> > Apologies again for the wrong mail account used in my previuos mail.
> 
> +			/* Remove the file from the property dialog */

fixed

> 
> This comment seems to be in the wrong place. (Not your fault really, it
> was in old code too.)
> 
> +		unique = 0;
> 
> Use NULL, not 0.

fixed, the culprit was #define NULL ((void *)0) in C++ code :/

> 
> 
> +		for (tmp = window->details->changed_files; tmp != NULL; tmp = tmp->next) {
> +			if (g_list_find (unique, tmp->data)) {
> +				nautilus_file_unref (NAUTILUS_FILE (tmp->data));
> +				continue;
> +			}
> 
> Do the uniqueification when adding the file instead. That way the
> changed_files list can't grow unbounded, and uniquification will be
> faster.

fixed.

> 
> 
> +
> +	directory_contents_value_field_update (window);
> [From properties_window_update()]
> 
> This is already called in update_contents_callback(), so this means this
> code is run twice in the callback.

fixed.

> 
> +	if (files == NULL) {
> +		dirty_original = TRUE;
> +		dirty_target = TRUE;
> +	}
> 
> This is used for the initial refresh everything, but it means it does
> the whole properties_window_update() even if only
> updated_deep_count_in_progress is needed.
> I think not refreshing if files == NULL, and instead passing TRUE in a
> dirty_all argument when doing the initial properties_window_update()
> would work better. To avoid calling
> directory_contents_value_field_update() unnecessary there should be a
> directory_contents_dirty flag too, which get set by the 
> "updated_deep_count_in_progress" signal handler.

I don't think this optimization is worth it: the code as-is works really
pretty well. If someone complains about performance, maybe we can fix it
later. If you don't agree, I can add that to the patch though...

-- 
Mathieu Lacage <mathieu gnu org>