duesee / imap-codec

Rock-solid and complete codec for IMAP
Apache License 2.0
38 stars 14 forks source link

docs: Update outdated README and check all code examples automatically #393

Closed coalooball closed 10 months ago

coalooball commented 10 months ago

Hello! I just downloaded imap-codec using cargo add imap-codec, and I want to run the examples from https://crates.io/crates/imap-codec, but I'm getting a bunch of errors.

Here are the error messages:

[coalooball /home/cyan/rs_test_project]
==># cargo t
   Compiling minimal-lexical v0.2.1
   Compiling base64 v0.21.5
   Compiling log v0.4.20
   Compiling num-traits v0.2.17
   Compiling thiserror v1.0.50
   Compiling nom v7.1.3
   Compiling chrono v0.4.31
   Compiling imap-types v1.0.0
   Compiling abnf-core v0.6.0
   Compiling imap-codec v1.0.0
   Compiling rs_test_project v0.1.0 (/home/cyan/rs_test_project)
error[E0432]: unresolved imports `imap_codec::codec::Decode`, `imap_codec::codec::Encode`
 --> src/main.rs:2:13
  |
2 |     codec::{Decode, Encode},
  |             ^^^^^^  ^^^^^^ no `Encode` in `codec`
  |             |
  |             no `Decode` in `codec`
  |
help: a similar name exists in the module
  |
2 |     codec::{decode, Encode},
  |             ~~~~~~
help: a similar name exists in the module
  |
2 |     codec::{Decode, encode},
  |                     ~~~~~~

error[E0603]: module `codec` is private
   --> src/main.rs:2:5
    |
2   |     codec::{Decode, Encode},
    |     ^^^^^ private module
    |
note: the module `codec` is defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/imap-codec-1.0.0/src/lib.rs:109:1
    |
109 | mod codec;
    | ^^^^^^^^^

error[E0603]: module `command` is private
   --> src/main.rs:3:5
    |
3   |     command::Command,
    |     ^^^^^^^ private module
    |
note: the module `command` is defined here
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/imap-codec-1.0.0/src/lib.rs:110:1
    |
110 | mod command;
    | ^^^^^^^^^^^

Some errors have detailed explanations: E0432, E0603.
For more information about an error, try `rustc --explain E0432`.
error: could not compile `rs_test_project` (bin "rs_test_project" test) due to 3 previous errors

Here is my source code(from https://crates.io/crates/imap-codec) and Cargo.toml(generated by cargo add imap-codec) file:

[coalooball /home/cyan/rs_test_project]
==># cat src/main.rs
use imap_codec::{
    codec::{Decode, Encode},
    command::Command,
};

fn main() {
    let input = b"ABCD UID FETCH 1,2:* (BODY.PEEK[1.2.3.4.MIME]<42.1337>)\r\n";

    let (remainder, parsed) = Command::decode(input).unwrap();
    println!("# Parsed\n\n{:#?}\n\n", parsed);

    let buffer = parsed.encode().dump();

    // Note: IMAP4rev1 may produce messages that are not valid UTF-8.
    println!("# Serialized\n\n{:?}", std::str::from_utf8(&buffer));
}[coalooball /home/cyan/rs_test_project]
==># cat Cargo.toml
[package]
name = "rs_test_project"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
imap-codec = "1.0.0"
duesee commented 10 months ago

Hello @coalooball! Thanks for submitting an issue!

That's what happens when code is not automatically checked... :-) The examples are outdated and need to be fixed.

In the meantime: Could you try the examples from https://docs.rs/imap-codec/latest/imap_codec/ and see if they work for you?

duesee commented 10 months ago

Alternatively, you might find these examples (https://github.com/duesee/imap-codec/tree/main/imap-codec/examples) helpful, too.

duesee commented 10 months ago

We could use https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html#include-items-only-when-collecting-doctests (see how to include README) to test the example in the README.

coalooball commented 10 months ago

Hello @duesee! Thanks for your prompt response.

I tried the examples from the two links you provided. I was able to run all the examples in this link (https://github.com/duesee/imap-codec/tree/main/imap-codec/examples). However, I encountered compilation errors when trying the examples from this link (https://docs.rs/imap-codec/latest/imap_codec/).

I need to execute the cargo add imap-types command in the project's root directory, and I also need to modify the code of this example like this:

use imap_codec::{decode::Decoder, GreetingCodec};
use imap_types::response::{Code, Greeting, GreetingKind};
use imap_types::core::Text;
fn main() {
    let (remaining, greeting) = GreetingCodec::default()
        .decode(b"* OK [ALERT] Hello, World!\r\n<remaining>")
        .unwrap();

    assert_eq!(
        greeting,
        Greeting {
            kind: GreetingKind::Ok,
            code: Some(Code::Alert),
            text: Text::try_from("Hello, World!").unwrap(),
        }
    );
    assert_eq!(remaining, &b"<remaining>"[..])
}

I think it's worth reminding everyone to use use imap_types::response::{Code, Greeting, GreetingKind}; cause I tend to instinctively use use imap_codec::response::{Code, Greeting, GreetingKind};.

By the way, my Cargo version is as follows:

cargo --version
cargo 1.75.0-nightly (6fa6fdc76 2023-10-10)
duesee commented 10 months ago

Thanks for your valuable input! I agree.

Currently, imap-codec re-exports imap-types and you could use imap_codec::imap_type::". BUT: I think this is confusing, and like to remove the re-export at some point.

Thus, I completely agree that the examples should mention to cargo add imap-types, too, and tell what to use.

As you already fixed it... are you in a position to make a PR? :-) I can do it, too, but as you already figured this all out, you should take the credit!

duesee commented 10 months ago

We could make two PRs: One fixing the outdated README (and making it so that it is automatically checked) -- I can do this PR quickly -- and one improving the examples -- possibly you? :-)

duesee commented 10 months ago

I made a PR :-) This commit https://github.com/duesee/imap-codec/pull/395/commits/953ce6b7c6051bc76f6d14acf49831ec7c4d4d7d will ensure that all examples in the READMEs are tested. So, hopefully, nothing can get out-of-sync anymore!

duesee commented 10 months ago

Reopened: Missed one README :P

coalooball commented 10 months ago

Thank you very much for the CREDIT :) I have already submitted the PR for #394.

In addition, I think re-exports is a good idea as well. To avoid duplication, I suggest that use imap_codec::imap_types::* could be changed to use imap_codec::types::*. Of course, it can also be use imap_types::*, depending on your preference.

duesee commented 10 months ago

I think at some point we re-exported imap-types as types. But I felt it blurred that the whole imap-types crate is just re-exported. The duplication is still unfortunate, though.