Author Topic: I'm implementing tags (aka labels)  (Read 15935 times)

Offline jauricchio

  • Newbie
  • *
  • Posts: 9
I'm implementing tags (aka labels)
« on: August 09, 2006, 01:46:05 AM »
Hi!

I'm fed up with the fact that there are, to my knowledge, no webmail systems that support tags/labels (with the notable exception of GMail). So I'm going to do something about it.

I'm going to add tags to Roundcube! =]

I intend to read tags from, and store them as, IMAP Flags. The result should be somewhat interoperable with Thunderbird and Pine. (I've never used Pine, so I won't be able to test interop; I'm not a Thunderbird user but I have installed it and played with its tagging.)

Now to the nitty-gritty.

The plan, so far, is:
- clean up the way flags are handled in rcube_imap.inc and lib/imap.inc. They're up-cased, and "SEEN", "DELETED", etc are magical. I believe a better way to do this is to preserve flag case all the way through, and use "\Seen", "\Deleted", etc as IMAP does, including the backslash. Dealing with the backslash will be a bit icky on the PHP side, but it should be doable.
- add API for working with arbitrary tags. I think I'll start with steps/mail/mark.inc. I need to investigate exactly what this file is and when it gets called, though.
- UI for tags. Not sure whether to clone GMail or what. Input would be especially appreciated here, I have a little UI-fu but I'm not a designer.

I'll report my progress here. I welcome all questions, comments, suggestions, corrections, and flames!

Offline jrmy

  • Jr. Member
  • **
  • Posts: 48
Re: I'm implementing tags (aka labels)
« Reply #1 on: August 09, 2006, 02:02:12 AM »
Sounds like a very handy feature. :)

I have never used lables in anything but gmail but I *hate* how it handles them. Having just a huge list run down the side is horrible if you ask me.


As for pine once you get to the point of worring about it I could probably test it out. I used pine for many years and still do on a few accounts.

Also, you might want to get on the developer mailing list. I don't *think* many developers post here and someone *may* already be working on this although I doubt it.

Good luck!

Offline EricS

  • Jr. Member
  • **
  • Posts: 45
Re: I'm implementing tags (aka labels)
« Reply #2 on: August 09, 2006, 10:51:47 AM »
jauricchio,

I'm all for this! For reference, take a look at:

http://trac.roundcube.net/trac.cgi/ticket/1455740
http://lists.roundcube.net/mail-archive/roundcube.dev/2006/07/48/

Let me know if you want any help!
-Eric

Offline jauricchio

  • Newbie
  • *
  • Posts: 9
Re: I'm implementing tags (aka labels)
« Reply #3 on: August 09, 2006, 04:10:58 PM »
Thanks jrmy, EricS!

There are roughly 92 lines that use capitalized strings like UNSEEN, DELETED, etc with magical meaning.

I'm going to go through them and write up testcases to make sure I exercise them all; then I'll start changing things to \Unseen, \Deleted, etc, and test.

Offline EricS

  • Jr. Member
  • **
  • Posts: 45
Re: I'm implementing tags (aka labels)
« Reply #4 on: August 09, 2006, 04:22:06 PM »
Be careful when you remove the "magic" string literals. You are correct that "\Deleted" and "\Seen" are the flag values, but the keywords "DELETED" and "SEEN" are the parameters used in a SEARCH command that refers to those flags.

See roundcubemail/program/lib/imap.inc for RoundCube's implementation of the IMAP client interface, including flags and searches.

-Eric

Offline jauricchio

  • Newbie
  • *
  • Posts: 9
Re: I'm implementing tags (aka labels)
« Reply #5 on: August 09, 2006, 05:17:31 PM »
Thanks for the heads-up.

However I change things, lib/imap.inc will have the mappings into whatever IMAP wants to hear, be it flag names or search criteria, and everything above it will have one common way to say "unseen". This will take some thought.


Is it true that all IMAP goes through include/rcube_imap.inc and then lib/imap.inc? Are there any other paths? Any code from above that short-circuits rcube_imap?

Offline EricS

  • Jr. Member
  • **
  • Posts: 45
Re: I'm implementing tags (aka labels)
« Reply #6 on: August 09, 2006, 05:56:56 PM »
Right now the client code passes flag values of 'read' and 'unread' to the server (see program/js/app.js:toggle_read_status). The server has a mapping from those "client" flag values to the "server" flag values (see program/steps/mail/mark.inc). These flag values are then given to the imap code (program/lib/imap.inc), wherein there is a mapping to the "real" IMAP flag values (check out the "iil_C_ModFlag" function).

The different flags provide a nice level of decoupling, but it might be nice to use consistent flag terminology within the client/JavaScript code and the RoundCube server code.

It looks like iil_C_ModFlag might already do most of what you want, you would just have to prevent it from doing the translation in cases where an actual IMAP Flag ("\\[A-Z][a-z]+") is provided.

Have fun!
-Eric

Offline jauricchio

  • Newbie
  • *
  • Posts: 9
Re: I'm implementing tags (aka labels)
« Reply #7 on: August 09, 2006, 07:07:11 PM »
I was also thinking of completely special-casing the few magical tags. imap.inc has a handful of convenience functions to mark messages as seen, unseen, etc. I could duplicate those at the rcube_imap.inc level. All magical operations (seen, unseen, draft, ...undraft?, deleted, undeleted, answered, unanswered, flagged, unflagged) become function calls, and all uses of tags are

Now, this makes searching kinda tricky, since as you pointed out "UNSEEN" is a search criterion. These could be special-cased into functions too.

$IMAP->messagecount() is already special-cased to hell. Its modes of messagecount() are ALL, RECENT, and UNSEEN, mutually-exclusive. It seems to me perfectly reasonable to split it up into messagecount_all, messagecount_recent, and messagecount_unseen. (The names are a bit clunky, any suggestions? =])

However, defining functions like that will make it very difficult to do certain complex searches in the future - "UNSEEN AND DELETED", for instance, would be impossible without writing a new function specifically for that. That functionality will not be easily available to users. It's not currently present at all, but it may be a concern in the future. I'll leave it up to those who do strategy: Do we want to support complex searches like that in the future?

--
Does anybody know why RECENT is an option to ill_C_ModFlag? RFC2060 section 2.3.2 specifically says "This flag can not be altered by the client.". I'm going to remove it as part of my changes, I think.

Offline jauricchio

  • Newbie
  • *
  • Posts: 9
Re: I'm implementing tags (aka labels)
« Reply #8 on: August 10, 2006, 01:18:47 PM »
I've been doing it wrong.

My task, as a feature implementor, is to add not modify; to change the existing codebase as little as possible. Rearchitecting should be left to others.

So I'm just going to duplicate iil_C_ModFlag to pass its arg directly through - no upcasing, no magical translation - and create a new flag function in rcube_imap.inc for arbitrary flags.

Offline EricS

  • Jr. Member
  • **
  • Posts: 45
Re: I'm implementing tags (aka labels)
« Reply #9 on: August 10, 2006, 01:50:06 PM »
Or you could add a small bit of logic to iil_C_ModFlag to decide whether to do the translation of the incoming flag. The decision could either be based on an explicit (new) function parameter or on pattern-matching of the flag (i.e. does it begin with "\"?).

Offline jauricchio

  • Newbie
  • *
  • Posts: 9
Re: I'm implementing tags (aka labels)
« Reply #10 on: August 10, 2006, 02:25:53 PM »
Yeah, that makes sense. I'll make ill_C_ModFlag a little bit smarter, and I'll have everything in rcube_imap.inc that calls it use the magic flags when appropriate. I'll provide convenience functions in $IMAP's class (rcube_imap.inc) for changing the magic flags, but I won't change any code to use them yet.


--update
I have really, really got to read through this code more before I start changing things!
It turns out the code DOESN'T directly call iil_C_ModFlag with the magical capitalized strings! $IMAP->set_flag seems to be a spof, and it has a bit of logic to call magic functions.

New plan: (dizzying how fast things change, isn't it!)
- add more magic-flag functions to lib/imap.inc
- $IMAP->set_flag will use the magic-flag functions for all its operations
- iil_C_ModFlag will preserve its argument, including backslashes and case
- magic-flag functions will user proper backslashes and case
- add $IMAP->label_message and $IMAP->unlabel_message, which will call newly modified iil_C_ModFlag --update: redefining the semantics for iil_C_Flag and _Unflag to do this.

--update
Things to do later
- make drafts set \Draft flag!
- do we set \Answered?

Offline EricS

  • Jr. Member
  • **
  • Posts: 45
Re: I'm implementing tags (aka labels)
« Reply #11 on: August 10, 2006, 03:41:18 PM »
That sounds like a good approach. Let me know when you have something to look at, and I'll be happy to give it a 2nd look! :)

-Eric

Offline jauricchio

  • Newbie
  • *
  • Posts: 9
Re: I'm implementing tags (aka labels)
« Reply #12 on: August 10, 2006, 06:33:47 PM »
Here's a patch for the first two steps.

Summary of changes:

include/rcube_imap.inc
- $IMAP->set_flag is now solely intended for magic flags.
- set_flag uses a map from strings like "SEEN" to function names like iil_C_Seen, to determine which function to call, and call_user_func to make the call.
- At the end of set_flag, there is an if/elseif/elseif to update the cached messagecount. All three branches of the if checked $result. Now, there is an if($result) wrapping them. This will save like two cycles =D

lib/imap.inc
- Existing functions iil_C_Unseen, _Delete, and _Undelete reordered =]
- Added functions iil_C_Draft, _Undraft, _Answered, _Unanswered, _Seen; which twiddle the appropriate IMAP magic flag. They call iil_C_ModFlag.

These are internal changes; there should be no detectable change in program operation.

I've tested this briefly with mark seen/unseen, and it seems to work. I'd appreciate a quick review from EricS or anyone else following this thread. Thanks!

(btw, could someone allow uploads of .patch files? .patch.txt is silly =])

Offline jauricchio

  • Newbie
  • *
  • Posts: 9
Re: I'm implementing tags (aka labels)
« Reply #13 on: August 10, 2006, 10:05:16 PM »
Forgot \Flagged.
Use this patch instead.

Offline jauricchio

  • Newbie
  • *
  • Posts: 9
Re: I'm implementing tags (aka labels)
« Reply #14 on: August 10, 2006, 10:27:40 PM »
Stage two complete:

- iil_C_ModFlag performs no translation on the flags it is passed as input. Validation is left to the caller. Spaces will cause multiple flags to be affected; stray parentheses are probably a very bad idea.
- iil_C_Seen, _Unseen, and so forth now use proper \Seen etc flags
- iil_C_FetchHeaders pulls all the flags without translation - it doesn't drop the backslashes
- iil_C_Flag and _Unflag are now generalized - they will be used for tags!

Once again, this should cause no detectable change in operation.

This patch depends upon, and includes, the previous patch ("clean_up_magic_flags_secondtry.patch.txt". I know, I need better names.)

This oughta be the end of the internal re-jiggering. From now on it's all new code!