Re: [evolution-patches] Re: Improve folder treeview order

[Not Zed <notzed ximian com>]

On Tue, 2004-06-01 at 21:18 +0200, Christian Neumair wrote:
> Christian Neumair schrieb: 
>
> > The attached patch improves the sort order of folders inside the folder 
> > treeview. See patch for order details. 
>
> Sorry, the patch was not only lacking the most important part, it didn't 
> work properly as well. Here comes the fixed patch. 
Some comments below.  Like Jeff, i'm not that sure we want to do this; although I don't feel particularly strongly either way.

Plain text document attachment (evo-treeview-order.diff)

Index: mail/em-folder-tree-model.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/em-folder-tree-model.c,v
retrieving revision 1.53
diff -u -r1.53 em-folder-tree-model.c
--- mail/em-folder-tree-model.c	28 May 2004 17:04:18 -0000	1.53
+++ mail/em-folder-tree-model.c	1 Jun 2004 19:17:12 -0000
@@ -177,6 +177,40 @@
 			      G_TYPE_STRING);
 }
 
+/* This makes folders show up in the following order: Inbox, Outbox, Drafts, Sent, Junk, Trash, Others */

Not quite what it does, it puts Trash before Junk :)

Personally i'd put junk and trash last of all, but maybe thats just me.

> +static guint
> +name_to_position (gchar *name)
> +{
> +	g_assert (name);
Why add an assert?  The old code checked for name NULL, it shouldn't change behaviour.

> +	if ((!strcmp (name, "INBOX") ||
> +	     !strcmp (name, "Inbox") ||
> +	     !strcmp (name, _("Inbox"))))
> +		return 0;
> +	else if ((!strcmp (name, "OUTBOX") ||
> +		  !strcmp (name, "Outbox") ||
> +		  !strcmp (name, _("Outbox"))))
> +		return 1;
> +	else if ((!strcmp (name, "DRAFTS") ||
> +		  !strcmp (name, "Drafts") ||
> +		  !strcmp (name, _("Drafts"))))
> +		return 2;
> +	else if ((!strcmp (name, "SENT") ||
> +		  !strcmp (name, "Sent") ||
> +		  !strcmp (name, _("Sent"))))
> +		return 3;
> +	else if ((!strcmp (name, "TRASH") ||
> +		  !strcmp (name, "Trash") ||
> +		  !strcmp (name, _("Trash"))))
> +		return 4;
> +	else if ((!strcmp (name, "JUNK") ||
> +		  !strcmp (name, "Junk") ||
> +		  !strcmp (name, _("Junk"))))
> +		return 5;
> +	else
> +		return 6;
I will say one word, well two.  Tables.  Loops. :)  There's no reason at all for a nested if block here.

I would probably use negative numbers for the indexes too, and 0 for not found/not special.  Fewer places to change if the list changes.

> +}
> +
>  static int
>  sort_cb (GtkTreeModel *model, GtkTreeIter *a, GtkTreeIter *b, gpointer user_data)
>  {
> @@ -184,6 +218,7 @@
>  	char *aname, *bname;
>  	CamelStore *store;
>  	gboolean is_store;
> +	guint atype, btype;
>  	
>  	gtk_tree_model_get (model, a, COL_BOOL_IS_STORE, &is_store,
>  			    COL_POINTER_CAMEL_STORE, &store,
> @@ -207,11 +242,12 @@
>  		if (bname && !strcmp (bname, _("UNMATCHED")))
>  			return -1;
>  	} else {
> -		/* Inbox is always first */
> -		if (aname && (!strcmp (aname, "INBOX") || !strcmp (aname, _("Inbox"))))
> -			return -1;
> -		if (bname && (!strcmp (bname, "INBOX") || !strcmp (bname, _("Inbox"))))
> -			return 1;
> +		atype = name_to_position (aname);
> +		btype = name_to_position (bname);
> +
> +		if (atype < 6 ||
> +		    btype < 6)
> +			return (atype == btype ? 0 : atype > btype ? 1 : -1);
I prefer the GNU if syntax:
   blah
 || blah
although for such a short _expression_ i'd put it on the one line.
Also its better to check atype < btype   atype > btype, since they're almost never going to be ==.

--

Michael Zucchi <notzed ximian com> Ximian Evolution and Free Software Developer