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



On Thu, 2004-06-03 at 16:17 +0200, Christian Neumair wrote:
Am Mi, den 02.06.2004, 19:35 Uhr +0800 schrieb Not Zed:
> On Wed, 2004-06-02 at 12:11 +0200, Christian Neumair wrote:  
> > Am Mi, den 02.06.2004, 11:22 Uhr +0800 schrieb Not Zed:
> > > 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.
> > 
> > The reason I want to do this is that in locales != C, all the folders
> > are shuffled randomly, because they can be partly untranslated. If you
> > change the locale, you'll get a new shuffe. This looks heavily
> > inconsistent and causes confusion, because people remember "it was on
> > top", "it was the nth item", "it was the first item with a folder icon".
> > Ultimately, it looks ways more nice, see "[evolution-patches] Make
> > folder treeview internationalization work" [1].
> 
> How many people really change their LC_MESSAGES all the time?  If the
> answer isn't "bugger all" then it makes sense.  Anna?  I guess it
> makes sense if you change it, for spatial memory.

Its not a matter of changing environment variables. You simply expect an
application to be consistent among all locales when it comes to the
order of fundamental UI elements (besides RTL swap magic of course).

It isn't a fundamental ui element.  Its a sorted list.  A locale which sorts differently should sort differently.

> > > > 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.
> > 
> > I wasn't sure on the last two.
> I guess its all kinda arbitrary. 

On the order of Drafts and Sent: They seem to be special folders. You
can set them arbitrarily according to the prefs dialog. Not sure whether
that is useful at all and whether that means that we shouldn't handle
them specially here compared to normal folders.
On Junk/Trash: I'm not sure which of them is used more frequently. I've
swapped them in the attached version. Note that with the current
semantics, we can put them in the end without a problem as well. We
simply need to change the position members of the struct.

> > > > +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.
> > 
> > why does one add treeview rows that have no label. That doesn't make
> > sense and in my opinion hints at broken code.
> I don't know, and yes you may be right, but since the code did that
> before, its just better not to change the behaviour.

OK, I've changed that.

> > > > +	if ((!strcmp (name, "INBOX") ||
> > > > [...20 lines of junk...]
> > > > +		return 6;
> > > I will say one word, well two.  Tables.  Loops. :)  There's no reason
> > > at all for a nested if block here.
> > 
> > Ok, ok. I suck. I don't know why I used these stupid semantics :).
> Well, as a first cut/test, it worked anyway :)

I've revamped the patch and heavily cleaned up the sort function.
Besides, it's now extensible through the arrays. Note that the old
treeview leaked. Somebody forgot to free the fetched variables.

regs,
 Chris
--
Michael Zucchi <notzed ximian com>

Ximian Evolution and Free Software Developer


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]