Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Apr 29 20:39

    delete-merged-branch[bot] on add-v2-config-file

    (compare)

  • Apr 29 20:38

    djc on main

    Upgrade to GitHub-native Depend… (compare)

  • Apr 29 20:38
    djc closed #123
  • Apr 29 19:08
    codecov[bot] commented #123
  • Apr 29 19:06
    dependabot-preview[bot] labeled #123
  • Apr 29 19:06
    dependabot-preview[bot] opened #123
  • Apr 29 19:06

    dependabot-preview[bot] on add-v2-config-file

    Upgrade to GitHub-native Depend… (compare)

  • Apr 19 23:50
    mordak commented #120
  • Apr 19 20:12
    djc commented #120
  • Apr 18 20:01
    mordak commented #120
  • Apr 13 17:36
    djc closed #122
  • Apr 13 17:35
    dario23 commented #122
  • Apr 13 17:27
    stavteklassi synchronize #122
  • Apr 13 17:26
    stavteklassi opened #122
  • Apr 12 19:19
    dependabot-preview[bot] commented #121
  • Apr 12 19:19

    dependabot-preview[bot] on cargo

    (compare)

  • Apr 12 19:19
    djc closed #121
  • Apr 12 19:19
    djc commented #121
  • Apr 12 04:13
    dependabot-preview[bot] labeled #121
  • Apr 12 04:13
    dependabot-preview[bot] opened #121
James
@avitex
@djc Want me to pull in assert_matches for this PR?
Example case
assert_matches!(
            super::capability_data(b"CAPABILITY IMAP4rev1\r\n"),
            Ok((_, capabilities)) => {
                assert_eq!(capabilities, vec![Capability::Imap4rev1])
            }
        );
James
@avitex
@djc Pushed the changes up :)
James
@avitex
Night all, better get bed before it's day. @djc Have fun with the PR :) I'll have a look at any required changes tomorrow/later today.
James
@avitex
@jonhoo You wouldn't have any marvelous ideas would you? https://github.com/djc/tokio-imap/pull/48#discussion_r279074302
Jon Gjengset
@jonhoo
When in doubt use the same names as the spec: https://tools.ietf.org/html/rfc2045#section-2
Message has all the completely shared fields, and consist of one or more Entitys, which have the mostly-shared fields. An Entity has an enum Body, which can be one of Body::Basic, Body::Message, and Body::Multipart. The text type is a special case of Body::Basic.
James
@avitex
@jonhoo Thanks a ton, I'll look further into that
Jon Gjengset
@jonhoo
:)
James
@avitex
@djc @jonhoo Sorry for the late update! Would you two like to have a peek at what I've chosen to do with the dedup? https://github.com/djc/tokio-imap/blob/2e65088e0c9498fb74f04fc00b6ac4b946fc0d78/imap-proto/src/types.rs#L149
Including you Jon, as your crate is upstream from this :) Cheers!
Dirkjan Ochtman
@djc
looks okayish
can we lose the content_ prefixes on fields? these don't seem to add much
also I think we could just inline BodyStructureBasic et al into BodyStructure as struct variants
James
@avitex
Sure
Dirkjan Ochtman
@djc
why is Envelope boxed?
and is there a reason you didn't end up using another crate for the mime types?
James
@avitex
My reasoning behind the verbosity behindcontent_ was keeping it specific to content headers
Was to keep the struct a bit lighter, though probably not needed
Was going to save that for another PR, but I can start looking into it
I personally like the tuple variant style, nice to match on
Though it's not really that hard to { .. }
Dirkjan Ochtman
@djc
I find that in this case the increased indirection with all the similarly named types makes it harder to follow the code
different PR for mime types is okay
James
@avitex
Fair enough
Dirkjan Ochtman
@djc
also type -> ty
and sub_type just subtype
but at this point I'm nitpicking so that's a good sign :)
James
@avitex
I'm fine with nitpicking haha :p
Might as well get it right the first time
Though if you want to use type i'll have to escape them like r#type
As it's a reserved keyword
Dirkjan Ochtman
@djc
sorry, no I meant using ty instead of typ
James
@avitex
Oh my bad
Dirkjan Ochtman
@djc
yeah, that library would probably be good
James
@avitex
If I use the above crate, we don't need to care anyways
Dirkjan Ochtman
@djc
would also be nice if the parser types could match the types types where possible
even better :)
James
@avitex
The parser types in body_structure?
The private ones that are there follow the RFC grouping and naming
Ah that mime crate is all locked down. Can't build it from parts, only parse from str from what I can see :(
Dirkjan Ochtman
@djc
okay, let's just leave it the way you have now
ok, let's keep them on the RFC grouping and naming then :)
James
@avitex
Leave it named as ContentType, or would you prefer Mime?
Dirkjan Ochtman
@djc
no, ContentType is fine
James
@avitex
And rename ContentDispositions typ -> ty inline with ContentType?
James
@avitex
I'll assume so, see the pushed changes :)
Dirkjan Ochtman
@djc
so is it ready for another review round?
Dirkjan Ochtman
@djc
@jonhoo would you mind if I make imap-proto rust 2018?
Jon Gjengset
@jonhoo
@djc seems fine to me :)
Dirkjan Ochtman
@djc
djc/tokio-imap@83b2890 fails because I can't figure out how to have body_structure.rssee the macros in parser::macros, anybody have any clues?