-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
imap, doc: documented stuffs on imapsession #2
base: master
Are you sure you want to change the base?
Conversation
|
||
* port Number, the port number of IMAP server | ||
* hostname String, the hostname of IMAP server | ||
* option Object, you config the authenticate option here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to document what option can contain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
before showing the example.
You should split README.md in two:
|
Do you want me to merge it and make progress in an other pull request? |
Ok, thanks, good idea :)
In this Pull Request, we should fix the whole TODO listed below IMO, so may i ask u merge this once all the And I'd like to do your comments tomorrow :) |
fixed something and still leaves few questions:
Can I create a new class named: var msg = new IMAPMessage();
msg.date = new Date();
msg.flags.push('\Seen');
// ...some sets to `msg`
session.appendMessage(folder, message);
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: How about this? I need your review again, @dinhviethoa |
For fetch options, in mailcore2, there's a bunch of options in a binary mask:
We could map them in JS to something like |
then how we handle the |
I'd suggest to do always PEEK. And let people mark explicitly as read with a storeFlags() API. |
How do u think the naming of the element of the option? I prefer use |
Ok, updated document for fetch functions, if it good to u, i will update it to source code :) |
and delete(complete) 3rd task in the top of this conversation. |
Sounds good to me. |
|
||
* folder String | ||
* data String, rfc822 message data | ||
* flags Array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make flags optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep
On Wed, Jul 2, 2014 at 12:47 AM, Hoà V. DINH [email protected]
wrote:
In docs/API.md:
+* text
+
+Otherwise like RFC3501, you should assign theoption
toALL
,FAST
orFULL
to do a shortcut.
+
+#### imap.search(query, callback)
+
+* query String
+* callback Function
+
+TheSEARCH
command implementation.
+
+#### imap.appendMessage(folder, data, flags, callback)
+
+* folder String
+* data String, rfc822 message data
+* flags ArrayCan 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
Ok, done :-) |
|
||
* folder String | ||
* data String, rfc822 message data | ||
* flags Array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you document or link to some possible values for flags?
* extra-headers | ||
* size | ||
|
||
Otherwise like RFC3501, you should assign the `option` to `ALL`, `FAST` or `FULL` to do a shortcut. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this line. We don't want people to use ALL, FAST, FULL, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You still need to document the return value of fetchMessages...() in the callback.
Ok, fix some comments :-) |
* 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't it be ['uid', 'flags', {'custom-headers': ['X-Mailer', 'X-Enveloper-Sender']}]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, i think it is too nested :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about: 'uid', 'flags', 'custom-header:X-Mailer', 'custom-header:X-Mailing-List'
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are custom header that contain a X-Prefix. And some headers are not listed as part of the "headers" or "full-headers".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for example, how do i add the x-mailer
to the fetchAtt
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, i see
Also, you should start implementation. |
I documented some stuffs and now updates the TODO List:
Documentation of each method in IMAPSession.js, parameters and examples, returned data.remove options parameter of copyMessages: use only UID.LGTY?