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

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

Closed
coalooball opened this issue Dec 5, 2023 · 10 comments · Fixed by #395 or #399
Closed

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

coalooball opened this issue Dec 5, 2023 · 10 comments · Fixed by #395 or #399
Labels
bug Something isn't working documentation Improvement of documentation good first issue Good for newcomers

Comments

@coalooball
Copy link
Contributor

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
Copy link
Owner

duesee commented Dec 5, 2023

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
Copy link
Owner

duesee commented Dec 5, 2023

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

@duesee duesee added bug Something isn't working documentation Improvement of documentation good first issue Good for newcomers labels Dec 5, 2023
@duesee
Copy link
Owner

duesee commented Dec 5, 2023

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
Copy link
Contributor Author

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
Copy link
Owner

duesee commented Dec 6, 2023

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
Copy link
Owner

duesee commented Dec 6, 2023

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 duesee changed the title v1.0.0 error[E0432]: unresolved imports imap_codec::codec::Decode, imap_codec::codec::Encode docs: Update outdated README and check all code examples automatically Dec 6, 2023
@duesee
Copy link
Owner

duesee commented Dec 6, 2023

I made a PR :-) This commit 953ce6b will ensure that all examples in the READMEs are tested. So, hopefully, nothing can get out-of-sync anymore!

@duesee
Copy link
Owner

duesee commented Dec 6, 2023

Reopened: Missed one README :P

@coalooball
Copy link
Contributor Author

coalooball commented Dec 7, 2023

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
Copy link
Owner

duesee commented Dec 7, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvement of documentation good first issue Good for newcomers
Projects
Status: Done
2 participants