-
Notifications
You must be signed in to change notification settings - Fork 2
imap, doc: documented stuffs on imapsession #2
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
base: master
Are you sure you want to change the base?
Conversation
README.md
Outdated
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. |
docs/API.md
Outdated
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 theoptiontoALL,FASTorFULLto do a shortcut.
+
+#### imap.search(query, callback)
+
+* query String
+* callback Function
+
+TheSEARCHcommand 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 :-) |
docs/API.md
Outdated
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?
docs/API.md
Outdated
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 :-) |
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?