imap, doc: documented stuffs on imapsession #2

yorkie · 2014-06-29T06:19:04Z

I documented some stuffs and now updates the TODO List:

~~Documentation of each method in IMAPSession.js, parameters and examples, returned data.~~
appendMessage flags and date parameters should be made more nodeJS friendly + they should be made optional through an option parameter.
fetchMessagesByNumber / fetchMessagesByUid option parameter should also be made more nodeJS friendly.
~~remove options parameter of copyMessages: use only UID.~~
add tests(what i added)

LGTY?

dinhvh · 2014-06-29T17:25:58Z

README.md

+
+* port Number, the port number of IMAP server
+* hostname String, the hostname of IMAP server
+* option Object, you config the authenticate option here


Would be nice to document what option can contain.

before showing the example.

dinhvh · 2014-06-29T17:32:16Z

You should split README.md in two:

One to describe what is the project and show a quick example.
Documentation.md, which shows a more complete documentation.

dinhvh · 2014-06-29T17:32:45Z

Do you want me to merge it and make progress in an other pull request?

yorkie · 2014-06-29T18:30:57Z

You should split README.md in two

Ok, thanks, good idea :)

Do you want me to merge it and make progress in an other pull request?

In this Pull Request, we should fix the whole TODO listed below IMO, so may i ask u merge this once all the TODO has been strike.

And I'd like to do your comments tomorrow :)

yorkie · 2014-06-30T10:47:16Z

fixed something and still leaves few questions:

appendMessage flags and date parameters should be made more nodeJS friendly + they should be made optional through an option parameter.

Can I create a new class named: IMAPMessage and let appendMessage be called like:

var msg = new IMAPMessage();
msg.date = new Date();
msg.flags.push('\Seen');
// ...some sets to `msg`
session.appendMessage(folder, message);

fetchMessagesByNumber / fetchMessagesByUid option parameter should also be made more nodeJS friendly.

What i think the best way to write the option just passing the command directly, like:

session.fetchMessagesByUID(0, 1, 'HEADER.FIELDS (FROM TO SUBJECT DATE)');

and for fresh we provide a default value for options: ALL, then they don't care about the option should be, however if they need, see the RFC3501 to learn how to concat their fetch command.

How about this? I need your review again, @dinhviethoa
Thank you :)

dinhvh · 2014-07-01T00:25:35Z

IMAPMessage already stands for something else in mailcore2. It's the result of fetchMessage...().
I don't think it's a good idea to introduce a new data type for the purpose of appendMessage().

For fetch options, in mailcore2, there's a bunch of options in a binary mask:

        IMAPMessagesRequestKindUid           = 0, // This is the default and it's always fetched
        IMAPMessagesRequestKindFlags         = 1 << 0,
        IMAPMessagesRequestKindHeaders       = 1 << 1,
        IMAPMessagesRequestKindStructure     = 1 << 2,
        IMAPMessagesRequestKindInternalDate  = 1 << 3,
        IMAPMessagesRequestKindFullHeaders   = 1 << 4,
        IMAPMessagesRequestKindHeaderSubject = 1 << 5,
        IMAPMessagesRequestKindGmailLabels   = 1 << 6,
        IMAPMessagesRequestKindGmailMessageID = 1 << 7,
        IMAPMessagesRequestKindGmailThreadID  = 1 << 8,
        IMAPMessagesRequestKindExtraHeaders  = 1 << 9,
        IMAPMessagesRequestKindSize          = 1 << 10,

We could map them in JS to something like ['uid', 'fullheaders', 'gmail-labels'].

yorkie · 2014-07-01T05:58:53Z

then how we handle the PEEK in this option?

dinhvh · 2014-07-01T14:12:31Z

I'd suggest to do always PEEK. And let people mark explicitly as read with a storeFlags() API.

yorkie · 2014-07-01T15:12:18Z

How do u think the naming of the element of the option? I prefer use a-b for all, do you?

yorkie · 2014-07-01T15:19:40Z

Ok, updated document for fetch functions, if it good to u, i will update it to source code :)

yorkie · 2014-07-01T15:20:28Z

and delete(complete) 3rd task in the top of this conversation.

dinhvh · 2014-07-01T16:46:00Z

How do u think the naming of the element of the option? I prefer use a-b for all, do you?

Sounds good to me.

dinhvh · 2014-07-01T16:47:14Z

docs/API.md

+
+* folder String
+* data String, rfc822 message data
+* flags Array


Can we make flags optional?

Yep

On Wed, Jul 2, 2014 at 12:47 AM, Hoà V. DINH notifications@github.com
wrote:

In docs/API.md:

+* text
+
+Otherwise like RFC3501, you should assign the option to ALL, FAST or FULL to do a shortcut.
+
+#### imap.search(query, callback)
+
+* query String
+* callback Function
+
+The SEARCH command implementation.
+
+#### imap.appendMessage(folder, data, flags, callback)
+
+* folder String
+* data String, rfc822 message data
+* flags Array

Can we make flags optional?

—
Reply to this email directly or view it on GitHub
https://github.com/MailCore/mailcore.node/pull/2/files#r14415276.

Yorkie Neil
https://github.com/yorkie
+18600219033

yorkie · 2014-07-01T16:52:57Z

Ok, done :-)

dinhvh · 2014-07-02T21:48:32Z

docs/API.md

+
+* folder String
+* data String, rfc822 message data
+* flags Array


Could you document or link to some possible values for flags?

dinhvh · 2014-07-02T21:56:17Z

docs/API.md

+* extra-headers
+* size
+
+Otherwise like RFC3501, you should assign the `option` to `ALL`, `FAST` or `FULL` to do a shortcut.


Remove this line. We don't want people to use ALL, FAST, FULL, etc.

You still need to document the return value of fetchMessages...() in the callback.

yorkie · 2014-07-03T04:51:16Z

Ok, fix some comments :-)

dinhvh · 2014-07-06T06:45:01Z

docs/API.md

+* gmail-thread-id
+* size
+
+And the element of `option` could also be custom header string like `X-Mailer`, `X-Envelope-Sender`, `X-Mailing-List` and etc.


Couldn't it be ['uid', 'flags', {'custom-headers': ['X-Mailer', 'X-Enveloper-Sender']}]?

Ah, i think it is too nested :(

how about: 'uid', 'flags', 'custom-header:X-Mailer', 'custom-header:X-Mailing-List'?

the custom-header and X- prefix seems act a same actor? i'm not sure if there is a custom header that doesn't contain a X- prefix?

There are custom header that contain a X-Prefix. And some headers are not listed as part of the "headers" or "full-headers".

Hi, there is a problem on implementation:

fetchAtt = [{type:Constants.FetchEnvelope}, {type:Constants.FetchBodySection, param:'1.2'}]

This is a line that you write at libetpan.node, and could you provide a way of add custom header fetch item?

for example, how do i add the x-mailer to the fetchAtt?

Have a look at _fetchTypeToString() in imapbase.js. You can use {type: Constants.FetchBodySection, section:'HEADER.FIELDS (x-mailer mailing-list)'}.
I think the documentation in comments is wrong. It should be section and not param.

Thanks, i see

dinhvh · 2014-07-06T06:45:21Z

Also, you should start implementation.

imap, doc: documented stuffs on imapsession

204f2b5

dinhvh reviewed Jun 29, 2014
View reviewed changes

yorkie added 3 commits June 30, 2014 18:08

doc: split document -> README/API

fe68157

doc: fix some comments

4cc6bc4

imap: remove options parameter of copyMessages: use only UID.

93021c1

dinhvh reviewed Jul 1, 2014
View reviewed changes

dinhvh reviewed Jul 2, 2014
View reviewed changes

document the fetch* functions

4e5965e

dinhvh reviewed Jul 6, 2014
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

imap, doc: documented stuffs on imapsession #2

imap, doc: documented stuffs on imapsession #2

yorkie commented Jun 29, 2014

dinhvh Jun 29, 2014

dinhvh Jun 29, 2014

dinhvh commented Jun 29, 2014

dinhvh commented Jun 29, 2014

yorkie commented Jun 29, 2014

yorkie commented Jun 30, 2014

dinhvh commented Jul 1, 2014

yorkie commented Jul 1, 2014

dinhvh commented Jul 1, 2014

yorkie commented Jul 1, 2014

yorkie commented Jul 1, 2014

yorkie commented Jul 1, 2014

dinhvh commented Jul 1, 2014

dinhvh Jul 1, 2014

yorkie Jul 1, 2014

yorkie commented Jul 1, 2014

dinhvh Jul 2, 2014

dinhvh Jul 2, 2014

dinhvh Jul 2, 2014

yorkie commented Jul 3, 2014

dinhvh Jul 6, 2014

yorkie Jul 6, 2014

dinhvh Jul 6, 2014

yorkie Jul 6, 2014

dinhvh Jul 6, 2014

yorkie Jul 10, 2014

yorkie Jul 10, 2014

dinhvh Jul 10, 2014

yorkie Jul 11, 2014

dinhvh commented Jul 6, 2014

imap, doc: documented stuffs on imapsession #2

Are you sure you want to change the base?

imap, doc: documented stuffs on imapsession #2

Conversation

yorkie commented Jun 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dinhvh commented Jun 29, 2014

dinhvh commented Jun 29, 2014

yorkie commented Jun 29, 2014

yorkie commented Jun 30, 2014

dinhvh commented Jul 1, 2014

yorkie commented Jul 1, 2014

dinhvh commented Jul 1, 2014

yorkie commented Jul 1, 2014

yorkie commented Jul 1, 2014

yorkie commented Jul 1, 2014

dinhvh commented Jul 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yorkie commented Jul 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yorkie commented Jul 3, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dinhvh commented Jul 6, 2014