Hi there, from a quick review it make sense.. I just notices you only did the changes for jpa mailbox yet and also the unit tests are broken.. Anyway I think it should work once this is sorted. One other thing I noticed while reading the diff and the imap rfc is the possibility of having a "NIL" seperator. We currently use a null String for this. With char we could use Character.UNASSIGNED for this..
Wdyt ? Bye, Norman 2011/1/5 Wojciech Strzałka <[email protected]>: > > Could you review the patch attached? It's a little bigger then > I'd expected. > > The central point for delimiter definition is > StoreMailboxManager::getDelimiter() which can be overloaded. Default > implementation uses MailboxConstants.DEFAULT_DELIMITER ( a '.' ) as a > delimiter. > All other references to this constant were removed. All other > separator definitions also. > > I'm not 100% sure of changes made to AbstractImapCommandParser. > New method decode(...) was introduced with default implementation > calling old method (to avoid changing all the CommandParser). Hovewer > abstract keyword was not removed from old implementation so the only > command implementing the new version (CreateCommandParser) needs to > provide old version method (an empty implementation). > > Don't hesitate to make any change requests or just throw it away if > you don't like it. > > I'm planning to test it a little in the next few days. > >> Hi there, > >> comments inside.. > >> 2011/1/3 Wojciech Strzałka <[email protected]>: >>> >>> I'm trying to implement it and I have a few questions: >>> >>> - it looks like there separate delimiters defined in 2 application parts: >>> a) mailbox separator (defined in MailboxConstants), >>> b) imap hierarchy delimiter (defined in ImapConstants) >>> >>> they interact with each other - so I guess they should be unified >>> to be the same after refactoring - am I right? > >> Exactly. I think the delimiter should be configured in the >> MailboxManager or MailboxSession (?) and then just looked-up in the >> imap processors. > > >>> >>> - delimiter is represented as 'String' in some places and as 'char' >>> in the others - should we unify this? Will be char enough? Are there >>> cases where it will be NULL (anyway should be probably represented as >>> chr(0)) or multi character string? > >> Yeah make this consistent is a good idea.. Char should be enough to >> hold the delimiter. The delimiter should never be NULL. > >>> >>> >>> >>> >>>> There are no stupid questions.... Just keep asking I will try to guide >>>> you as much As possible. >>> >>>> I think you will have to add the delimiter stuff to MailboxSession and >>>> replace the hardcoded Value with the One in MailboxSession >>> >>>> Good luck;) >>> >>>> Am Donnerstag, 30. Dezember 2010 schrieb Wojciech Strzałka >>>> <[email protected]>: >>>>> >>>>> I don't feel comfortable yet, with the overall project structure yet but >>>>> I can try :D >>>>> Expect stupid questions on priv :) >>>>> >>>>>> Good point, >>>>> >>>>>> I'm not sure I have time before 0.2 to implement this, but if you want >>>>>> and have time you could write a patch. We love contributions and I >>>>>> would be more then happy to review your work ;) >>>>> >>>>>> Bye, >>>>>> Norman >>>>> >>>>> >>>>>> 2010/12/30 Wojciech Strzałka <[email protected]>: >>>>>>> >>>>>>> ... a system where path separator is "/" and "." is allowed >>>>>>> character in folder name. >>>>>>> >>>>>>>> Hmm, >>>>>>> >>>>>>>> could you give me a use case for this ? Just try to understand why you >>>>>>>> want todo this.. >>>>>>> >>>>>>>> Bye, >>>>>>>> Norman >>>>>>> >>>>>>>> 2010/12/30 Wojciech Strzałka <[email protected]>: >>>>>>>>> >>>>>>>>> One of the missing pieces is possibility to change default server >>>>>>>>> hierarchy separator. Currently "." is fixed value. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> --------------------------------------------------------------------- >>>>>>>>> To unsubscribe, e-mail: [email protected] >>>>>>>>> For additional commands, e-mail: [email protected] >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>>> --------------------------------------------------------------------- >>>>>>>> To unsubscribe, e-mail: [email protected] >>>>>>>> For additional commands, e-mail: [email protected] >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Pozdrowienia, >>>>>>> Wojciech Strzałka >>>>>>> >>>>>>> >>>>>>> --------------------------------------------------------------------- >>>>>>> To unsubscribe, e-mail: [email protected] >>>>>>> For additional commands, e-mail: [email protected] >>>>>>> >>>>>>> >>>>> >>>>>> --------------------------------------------------------------------- >>>>>> To unsubscribe, e-mail: [email protected] >>>>>> For additional commands, e-mail: [email protected] >>>>> >>>>> >>>>> >>>>> -- >>>>> Pozdrowienia, >>>>> Wojciech Strzałka >>>>> >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe, e-mail: [email protected] >>>>> For additional commands, e-mail: [email protected] >>>>> >>>>> >>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: [email protected] >>>> For additional commands, e-mail: [email protected] >>> >>> >>> >>> -- >>> Pozdrowienia, >>> Wojciech Strzałka >>> > >> Bye, >> Norman > >> Ps: I CC server-dev as this is the right place for the mail thread.. > > > > -- > Pozdrowienia, > Wojciech Strzałka --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
