Skip to content
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

More involved parsing #28

Closed
jonhoo opened this issue Mar 2, 2017 · 10 comments · Fixed by #58
Closed

More involved parsing #28

jonhoo opened this issue Mar 2, 2017 · 10 comments · Fixed by #58

Comments

@jonhoo
Copy link
Collaborator

jonhoo commented Mar 2, 2017

Currently, all the commands simply return Vec<String>. It'd be great if they returned more semantically relevant results (i.e., fully parsed structs). This would also let us make the IDLE handle return an Iterator over said structs, so you could monitor changes to the mailbox in real-time.

@jonhoo
Copy link
Collaborator Author

jonhoo commented Mar 2, 2017

This also applies to arguments. For example, the sequence_set argument to fetch should probably be a Vec<Uid> or something, and should probably return something akin to a HashMap<Uid, _>.

@sanmai-NL
Copy link
Contributor

Would you be interested to cooperate with @djc on this? He has written another IMAP crate that supports nom-based server response parsing. I suppose a concerted effort is the best way forward. 🙂

@djc
Copy link

djc commented Nov 1, 2017

If you're interested we can probably split the protocol data structures and parser out of tokio-imap so that they can more easily be reused. In terms of encoding, so far I've mostly used str slices, although I recently changed the parser so that it returns bytes for message contents (for the RFC822 FETCH attribute only so far).

@jonhoo
Copy link
Collaborator Author

jonhoo commented Nov 1, 2017

@djc That would be awesome!

@djc
Copy link

djc commented Nov 2, 2017

The imap-proto crate is now live on crates.io: https://crates.io/crates/imap-proto.

@jonhoo
Copy link
Collaborator Author

jonhoo commented Nov 2, 2017

Docs at https://docs.rs/imap-proto/0.1.0/imap_proto/. Not too wordy atm, but still useful.
I don't know that I'll have time to look into incorporating this particularly soon, but it's on my radar. Otherwise, PRs welcome!

@djc
Copy link

djc commented Nov 2, 2017

I probably won't work on the integration between rust-imap and imap-proto, but if people find gaps in imap-proto for stuff that rust-imap, I can have a look at those.

@jonhoo
Copy link
Collaborator Author

jonhoo commented Nov 2, 2017

@djc yeah, I wasn't expecting that you would, just wanted to comment for others watching this issue :)
I might contribute some updates to the docs for imap-proto when I get around to incorporating it too.

@jonhoo
Copy link
Collaborator Author

jonhoo commented Nov 3, 2017

@djc from a cursory review, it looks like imap-proto expects all strings to be valid UTF-8. As has been pointed out elsewhere (see #54 for a summary), this can be a problem for some applications. Do you have a plan for the future for how you'll deal with servers that use other charsets?

@jonhoo
Copy link
Collaborator Author

jonhoo commented Nov 3, 2017

Filed djc/imap-proto#5 including a proposed solution.

mattnenterprise pushed a commit that referenced this issue Feb 9, 2018
* First stab at structured types (#28)

Tests currently fail due to djc/imap-proto#2.

LSUB is also broken due to djc/imap-proto#4 (but we don't have tests for
that atm).

* Also parse out RFC822 fetch responses

* Make all the things zero-copy

* Also delegate IntoIterator for ZeroCopy ref

* Fix UNSEEN and LSUB

All tests now pass

* Address @sanmai-NL comments

* Correctly handle incomplete parser responses

* No need for git dep anymore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants