olson-sean-k / wax

Opinionated and portable globs that can be matched against paths and directory trees.
https://glob.guide
MIT License
112 stars 10 forks source link

Miette integration seems not to be completed and other understandable behaviors #32

Open corebreaker opened 2 years ago

corebreaker commented 2 years ago

With Cargo file like this:

[package]
name = "wax-proto"
version = "0.1.0"
edition = "2021"

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

[dependencies]
miette = { version = "5.3.0", features = ["fancy"] }
wax = { version = "0.5.0", features = ["diagnostics", "diagnostics-report", "miette"] }

Note that, the Cargo file can be like this too, the problem will be the same:

[package]
name = "wax-proto"
version = "0.1.0"
edition = "2021"

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

[dependencies]
miette = { version = "5.3.0", features = ["fancy"] }
wax = { version = "0.5.0", features = ["diagnostics"] }

With the file, main.rs:

use miette::Result;
use wax::Glob;

fn main() -> Result<()> {
    Glob::new("**//*.txt")?;
    Ok(())
}

When we run the command cargo run: image

I found this result a little odd. With activated features in Cargo.toml, and https://github.com/olson-sean-k/wax/blob/master/src/lib.rs#L401, the response is understandable.

And that, even if we add the Diagnostic trait:

use miette::Diagnostic;
use miette::Result;
use wax::Glob;

fn main() -> Result<()> {
    Glob::new("**//*.txt")?;
    Ok(())
}

Plus, with that version of main.rs:

use miette::Diagnostic;
use wax::{Glob, GlobError};

fn main() -> Result<(), GlobError<'static>> {
    Glob::new("**//*.txt")?;
    Ok(())
}

We have this response: image

We don't have the expected fancy printing from Miette.

We can notice that in the Cargo.toml of Wax, here: https://github.com/olson-sean-k/wax/blob/master/Cargo.toml#L42, the fancy feature is not activated. Is it normal ?

To have a good use of the integration of Miette, there is something missing in our prototype project ?

olson-sean-k commented 2 years ago

I'm guessing the issue here concerns versioning and incompatible traits: version 0.5.0 of wax (the most recent published version at time of writing) integrates with miette version ^4.7.0. The example manifests that you've shared specify a dependency on version 5.3.0 of miette, so the traits are not compatible. You'll need to downgrade to a version specification of ^4.7.0 for miette if you're using version 0.5.0 of wax.

See also this comment on a related miette issue.

corebreaker commented 2 years ago

I knew this issue, but downgrade Miette doesn't fix the problem, you can check it by yourself. And these kind of problem on Miette integration has been kept once you remove the Miette dependency like that:

[package]
name = "wax-proto"
version = "0.1.0"
edition = "2021"

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

[dependencies]
wax = { version = "0.5.0", features = ["diagnostics"] }

With main.rs:

use wax::{Glob, GlobError};

fn main() -> Result<(), GlobError<'static>> {
    Glob::new("**//*.txt")?;
    Ok(())
}
olson-sean-k commented 2 years ago

I knew this issue, but downgrade Miette doesn't fix the problem, you can check it by yourself.

Hmm, that doesn't seem right. For what it's worth, I expect none of the example manifests you've shared so far to work properly with miette.

I have some working code that uses this integration, but I also created a minimal example just now and it works as expected. Are you sure you've taken an appropriate dependency specification on miette?

Here's a fairly minimal working manifest.

[package]
name = "wax_issue_32"
version = "0.0.0"
edition = "2021"

[dependencies]

[dependencies.miette]
version = "^4.7.0"
features = ["fancy"]

[dependencies.wax]
version = "^0.5.0"
features = ["diagnostics"]

I tested this with the following code and it prints diagnostics using miette's fancy reporter.

use wax::Glob;

fn main() -> miette::Result<()> {
    let _glob = Glob::new("**/{foo,**/bar}")?; // Error!
    Ok(())
}
corebreaker commented 2 years ago

Ok, thank you. Yes it works.

Could i suggest to put this example in documention of Wax cause for an user of Wax like me how to use Miette is not clear and we must read the documentation of Miette in addition of Wax.

Otherwise, you can close this issue if you want.

olson-sean-k commented 2 years ago

Great, I'm glad it's working!

Could i suggest to put this example in documention of Wax

Yes, this sounds like a good idea. I can include some details regarding versions of integrated traits and how to determine what version specifications to use. I'll keep this issue open to track this.