Vladislav Bogdanov wrote:

here is a most recent revision of a threaded view patch.

Most notable changes since rev4 (http://lists.roundcube.net/mail-archive/dev/2009-09/0000143.html):

1. Port to RC r3022
2. Add children of a last shift-selected message to selection too.

I don't see this message on the list, so I'm attaching the patch in my reply

Alec, would you please check this patch to not interfere with your last additions (message index) in a improper way?

Currently I have no time to test it well, but I've seen the patch and I've got a few thoughts:

1. threads support detection and thread algorithm auto-selection. I

think 'imap_thread_algorithm' should be set automatically to the best from those supported by server (REFS, REFERENCES, ORDEREDSUBJECT). It means if e.g. server supports REFS and ORDEREDSUBJECT we should use REFS. Does anybody need ORDEREDSUBJECT if he's server supports REFERENCES? Also see manage_folders.inc for: $IMAP->get_capability('thread=references');

2. Don't use massage_sort_col='default'. Since we have 'index_sort'

option. Messages are sorted by message index if message_sort_col='date'. We don't need this 'default' stuff also because THREAD command returns messages sorted by date.

3. We also wouldn't need enable_thread_caching option. Use

enable_caching instead. I know it will be hard now, but for me would be better (a much better) to remove the whole thread caching stuff from the patch and implement this in next step after threading will be working well without caching. For first step: enable_caching=true -> messages_treading=false. I'm of course a volunteer to implement the whole threads caching stuff.

4. Maybe list.js changes could be more universal. E.g. expand_unread()

could be implemented as expand_by_state('unread') or sth. The list.js is used not only for messages.

5. In mail steps use short $IMAP instead of

rcmail::get_instance()->imap->threading. Just for better readability.

Chris January wrote:

Assume imap_thread_algorithm='REFS'. Sometimes you want to sort by the date of the last message received in a thread (i.e. index_sort=true, message_sort_col='date') and sometimes you want to sort by the date of the first message in a thread (i.e. index_sort=false). At present the user must go into settings to turn index_sort on/off which is unintuitive. This is the reason I argued for a 4-state Date column.

I think 4-state switch isn't more intuitive.

...

5. In mail steps use short $IMAP instead of

rcmail::get_instance()->imap->threading. Just for better readability.

Agreed, but rcmail.php says:

• @todo Remove global $IMAP

and // set global object for backward compatibility $GLOBALS['IMAP'] = $this->imap;

Yes, but I think it's for roundcube classes. In steps scripts we can still use shortest $IMAP or at least $RCMAIL->imap.

Chris January wrote:

I just looked at what KMail does. It has both a Date column (for date of first message in thread) and Most Recent Date column (for date of most recent message in thread). Perhaps that's the way to go?

As I understand the first it's for ORDEREDSUBJECT and the letter for REFERENCES. If we need both we just should add an (working) option to change this behaviour. I think we don't need this (at least now) and we should just use default THREAD command sorting. Also probably not every server supports both.

Sorting by anything but the default order (i.e. index_sort=true) is incredibly slow with 30,000 or so messages.

THREAD command returns messages (threads) sorted by date, it's not possible to speed this up. Index_sort option will not help and we have two possibilities:

• re-sort by root message index if index_sort=true,
• forget about index_sort option when using threads

Assume imap_thread_algorithm='REFS'. Sometimes you want to sort by the date of the last message received
in a thread (i.e. index_sort=true, message_sort_col='date') and sometimes
you want to sort by the date of the first message in a thread (i.e. index_sort=false). At present the user must go into settings to turn index_sort on/off which is unintuitive. This is the reason I argued
for a 4-state Date column.

Hi, i just want to clarify something, because the above paragraph is
confusing me.

In my view index_sort has nothing to do with threading. index_sort
just says 'do I use the Date: header, or do I use the message index as
the sort order'. This helps for instance to prevent spammers from
inserting a date header far in the future being always sorted on top
(and at the same time it can increase performance). This can influence
both threaded and non-threaded message views. Is this correct or am i
misunderstanding what index_sort does?

If this is correct, then we need a totally seperate setting to tell us
how a user wants to sort a threaded message list and you cant just use
index_sort for that, because they're totally different things. We need
a setting that says 'do you want to sort your threaded message roots
based on first message in the thread of the newest message in the
thread'. And both of those could theoretically be influenced again by
the index_sort option. Say you want your threads sorted on first
message, then index_sort could change which message is first. Even
messages within a thread-level could be influenced by index_sort if
you really want to go that far. I do not agree with Chris that this
is something users want to change with a button. I believe the thread- root-sort-order (first or latest) is a personal preference that users
tend to set just once.

As another point, I do understand why Chris wants an unsorted view. In
dovecot this is really really fast, and I would also welcome such a
possibility.

Regards,

Cor _______________________________________________ List info: http://lists.roundcube.net/dev/

Chris January wrote:

You are right. There is index_sort which I do not personally need, but some users want and now it has been added to Roundcube - great.

However I have one mailbox which is very large and Roundcube takes a long time to sort the messages in it (threaded or not). For this mailbox I want to just see messages in the order the server sends them back, and not have Roundcube sort them at all because it is too slow.

Without threading if you use index_sort=true and sort_col=date, then you'll get what you want.

Cor Bosman wrote:

I totally agree, i have had this discussion with Alec before when I
suggested to use a SORT(ARRIVAL). But Alec then had a convincing
argument why it couldnt be used. Possibly this was waiting for the
implemention of index_sort? In which case maybe now it can be used?

"arrival sort" is better than implemented "index sort", but:

• I'm not really sure if "SORT(ARRIVAL)" is faster than "SORT(DATE)" on

every IMAP server implementation,

• "arrival sort" will be very slow on servers without SORT capability

(as it was before index_sort implementation).

I really don't know what "arrival sort" has in common with threading.

On Tue, 06 Oct 2009 15:05:06 +0300, Vladislav Bogdanov bubble@hoster-ok.com wrote:

Hi all,

here is a most recent revision of a threaded view patch.

Most notable changes since rev4 (http://lists.roundcube.net/mail-archive/dev/2009-09/0000143.html):

1. Port to RC r3022
2. Add children of a last shift-selected message to selection too.

Alec, would you please check this patch to not interfere with your last additions (message index) in a improper way?

Vladislav

Has anyone else tested this yet?

I am having some problems and have been discussing with Vladislav off-list however I am still having problems with a freshly installed 3022 so I would like to find out if it's just me or not.

The patch applies cleanly and if I enable threading for a folder in the Folders tab, it works great and the threading looks good.

However my problems are with non-threaded folders after the patch is applied. Sorting by ASC is normal, however sorting by DESC or the 3rd date-column state will sort with the newest page worth of emails on the first page, however on the page itself the mails seem to be ordered in reverse with newest emails on the bottom.

I have attached a screenshot showing the problem. I was just hoping someone could either confirm that the patch is working for them for threaded and non-threaded folders, or that they are seeing the same thing as me...

Cheers, Mark

--- 8< --- detachments --- 8< --- The following attachments have been detached and are available for viewing. http://detached.gigo.com/rc/nY/7CywnPye/sorting_example.JPG Only click these links if you trust the sender, as well as this message. --- 8< --- detachments --- 8< ---

List info: http://lists.roundcube.net/dev/

6. Threading code in rcube_imap class should be separated from

non-threading code a little bit more. E.g. _list_headers() is a mess.

+1

Please, consider my proposal from points 2, 3 and 6. Without this understandig of this part of patch (rcube_imap.php) is really hard.

Yep, it is always a headache for me too. Everything else is clean. But I do not understand underlying logic deep enough to deal with all that caching-not-caching-sorting-not-sorting... So it would be definitely better if you agree with Chris on how it should be and do it yourselves.

About the previous discussion: From my point of view 'default' ('unsorted') mode from a threading patch is almost the same as a 'index_sort' option, so it would probably be sufficient to just silently enable that option for threaded view just to prevent in-code resorting of what is already sorted by a server. I just missed that while merging patch last time.

And about overall usability of both features: More I think more I sure that there should be a per-folder sorting option for both threading and index_sort. Later should be enabled unconditionally when former is enabled. Either index_sort should be added as a column where threading is added or they both should be moved into a messagelist. It could be done as a narrow dropdown box before the 'subject' heading (at the horizontal level of expando boxes in a threaded view) with three options, 'enable threading', 'enable index_sort' and 'disable both'. Later should be a default. This solution is a preferred one for me if it is easy enough to implement this. It would add greater usability because it is often desirable to switch to another mode without visiting a settings pane... As I wrote before, new messages could appear on a third page of a threaded view (as it happened with me today on a RC Dev list ;) ) and it would be very convenient to switch to a plain mode in a just one click.

Vladislav

List info: http://lists.roundcube.net/dev/