Add support for RFC 4314/2086 ACL extension to imap-proto #142
Conversation
urkle
force-pushed
the
feat-acl-protocol
branch
2 times, most recently
from
99d6986
to
51d572e
Compare
imap-proto/src/types/acls.rs
Outdated
| } | ||
|
|
||
| #[derive(Debug, Eq, PartialEq)] | ||
| pub struct ACLRightList { |
There was a problem hiding this comment.
It's not obvious that we should have this type inside imap-proto. I think we should instead use a Vec<AclRight> which directly reflects what the server sent us.
There was a problem hiding this comment.
Ok.. I factored this out
urkle
force-pushed
the
feat-acl-protocol
branch
from
51d572e
to
e3f45b8
Compare
|
@djc thanks for the review.. I've updated the branch with your requested changes. |
imap-proto/src/parser/rfc4314.rs
Outdated
| let v: String = items.into_iter().flat_map(|s| s.chars()).collect(); | ||
|
|
||
| Ok((rest, map_text_to_rights(&v))) |
There was a problem hiding this comment.
This seems to have an intermediate allocation for no good reason. Can you rewrite to something that converts directly?
There was a problem hiding this comment.
yeah.. let me see if I can rework this.
There was a problem hiding this comment.
ok.. I reworked it to remove the String and to have it convert more directly.
imap-proto/src/types/acls.rs
Outdated
| } | ||
| } | ||
|
|
||
| pub(crate) type AclRightList = Vec<AclRight>; |
There was a problem hiding this comment.
I'd prefer to avoid the alias; IMO these usually make it harder to understand the code.
There was a problem hiding this comment.
yeah.. sorry that was supposed to be removed.. I have it removed now.
imap-proto/src/parser/rfc4314.rs
Outdated
| /// helper routine to map a string to a vec of AclRights | ||
| fn map_text_to_rights(i: &str) -> Vec<AclRight> { | ||
| i.chars() | ||
| .into_iter() |
There was a problem hiding this comment.
I think the type returned by chars() is an Iterator, so no need to call into_iter() on it?
|
Also I thought I wrote a comment about this, but please order the type definitions in the new module as top down. Most complex type (with dependencies on other types) first, simplest type ( |
urkle
force-pushed
the
feat-acl-protocol
branch
2 times, most recently
from
1968048
to
b82adb8
Compare
imap-proto/src/parser/rfc4314.rs
Outdated
| Ok(( | ||
| rest, | ||
| items.into_iter() | ||
| .flat_map(|s| map_text_to_rights(s)) |
There was a problem hiding this comment.
We need to resolve this lint still. I think the fix here is not to use map_text_to_rights() but to just use s.chars().map(|c| c.into()) directly, which would avoid allocating a Vec which will be dropped again immediately after.
ACL response LISTRIGHTS response MYRIGHTS response
urkle
force-pushed
the
feat-acl-protocol
branch
from
b82adb8
to
9d85627
Compare
|
@djc do you perhaps know when you'll make a new release of this crate? (so I can use it over in jonhoo/rust-imap#227 ) |
|
0.16.1 is on crates.io now! |
Implement parsing of the ACL response types specified in RFC 4314/2086 (and the Annotation right from RFC 5257)
ACL response
LISTRIGHTS response
MYRIGHTS response