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

paw compatibility + minor improv. #22

Closed
wants to merge 1 commit into from
Closed

Conversation

swfsql
Copy link

@swfsql swfsql commented Jul 8, 2020

I had an error to use this library in combination with structopt + paw, so I added paw as an optional dependency behind a feature.
Also added a minor improvement for Verbosity::set_default.

Comment on lines +18 to +22
paw = { version = "1.0", optional = true }

[features]
default = []
paw-compatibility = ["paw"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had an error to use this library in combination with structopt + paw, so I added paw as an optional dependency behind a feature.

How does having a paw dependency fix our support for paw?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added one demonstration as a new commentary, and I'm also now repeating what I placed in there, but I think that this is the case because on how structopt macros references paw.

Copy link
Contributor

@matthiasbeyer matthiasbeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These patches do not change anything.

Sorry to say, but I'd reject this.

@swfsql
Copy link
Author

swfsql commented Jul 9, 2020

I admit that this is surprising, but it does make a difference.
I made a repo to demonstrate this; you can check it by running:

git clone https://github.com/swfsql/clap-comp-example.git
cd clap-comp-example

git checkout afce9fd
cargo check

git checkout 89594fe
cargo check

git diff afce9fd

The main is just:

#[derive(structopt::StructOpt)]
struct Args {
    #[structopt(flatten)]
    verbose: clap_verbosity_flag::Verbosity,
}
#[paw::main]
fn main(args: Args) {}

The initial Cargo.toml is:

[package]
name = "example-a"
version = "0.1.0"
authors = ["Thi <swfsql@gmail.com.br>"]
edition = "2018"

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

[dependencies]
paw = "1.0.0"
structopt = { version = "0.3.0", features = ["paw"] }

[dependencies.clap-verbosity-flag]
version = "0.3.1"

(which fails to compile)

The final Cargo.toml is:

[package]
name = "example-a"
version = "0.1.0"
authors = ["Thi <swfsql@gmail.com.br>"]
edition = "2018"

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

[dependencies]
paw = "1.0.0"
structopt = { version = "0.3.0", features = ["paw"] }

[dependencies.clap-verbosity-flag]
version = "0.3.1"
features = ["paw-compatibility"]

[patch.crates-io.clap-verbosity-flag]
git = "https://github.com/swfsql/clap-verbosity-flag"
rev = "d9f92b7"

(which compiles), where in this I patched it to use the one from this PR.

I think that this is the case because on how structopt macros references paw.

@epage
Copy link
Member

epage commented Jul 9, 2020

Providing the error is helpful

error[E0433]: failed to resolve: use of undeclared type or module `paw`
  --> /Users/edpage/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-verbosity-flag-0.3.1/src/lib.rs:21:10
   |
21 | #[derive(structopt::StructOpt, Debug, Clone)]
   |          ^^^^^^^^^^^^^^^^^^^^ use of undeclared type or module `paw`

error: aborting due to previous erro

So structopt has been configured with paw but when processing the macro within clap-verbosity-flag, it only has visibility to the dependencies of clap-verbosity-flag.

Personally, I wish macros had an option to run as if they were coming from their own crate because they make it easy to break abstractions, like here. Either structopt needs to re-export paw like they do clap or every random structopt crate has to be aware of paw and provide a feature to satisfy the dependency.

Created an issue over at TeXitoi/structopt#407

@swfsql
Copy link
Author

swfsql commented Jul 10, 2020

I see! Thanks for creating the issue for them.

@swfsql swfsql marked this pull request as draft November 9, 2021 18:17
@swfsql swfsql closed this Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants