Compatibility with Clap 4?

[max-sixty]

Hi there! It's me again, from https://github.com/aj-bagwell/clio/issues/4

We're now attempting to upgrade to clap 4 (https://github.com/prql/prql/pull/999, though dependabot will probably open a new one for a new patch release).

I'm running cargo check --features clap/deprecated -p prql-compiler, and seeing messages like:

warning: use of deprecated function `<cli::CommandIO as clap::Args>::augment_args::parse_try_from_os_str`: Replaced with `#[clap(value_parser = ...)]` with a custom `TypedValuePa
rser`
  --> prql-compiler/src/cli.rs:42:55
   |
42 |     #[clap(default_value="-", parse(try_from_os_str = Input::try_from))]
   |                                                       ^^^^^
   |
   = note: `#[warn(deprecated)]` on by default

Does this mean it requires a TypedValueParser? Would this be something we should write or would it come from clio?

Thank you as ever, clio has served us well (and deserves more installs!)

[aj-bagwell]

I added support for TypedValueParser in 2.3 but it is hidden by the clap-parser feature as I did not want to have clap as a manditory dependecy.

I have just uploaded 2.4 that expands the clap versions it is compatible with in the Cargo.toml to include Clap 4.

You should be good with something along the lines of:

diff --git a/prql-compiler/Cargo.toml b/prql-compiler/Cargo.toml
index b9c0378..59e24c2 100644
--- a/prql-compiler/Cargo.toml
+++ b/prql-compiler/Cargo.toml
@@ -22,7 +22,7 @@ doctest = false
 anyhow = "1.0.57"
 ariadne = "0.1.5"
 atty = {version = "0.2.14", optional = true}
-clio = {version = "0.2.2", optional = true}
+clio = {version = "0.2.4", optional = true, features = ["clap-parse"]}
 color-eyre = {version = "0.6.1", optional = true}
 enum-as-inner = "0.5.0"
 itertools = "0.10.3"
diff --git a/prql-compiler/src/cli.rs b/prql-compiler/src/cli.rs
index 0e826ee..e5d150f 100644
--- a/prql-compiler/src/cli.rs
+++ b/prql-compiler/src/cli.rs
@@ -39,10 +39,10 @@ pub enum Cli {

 #[derive(clap::Args, Default)]
 pub struct CommandIO {
-    #[clap(default_value="-", parse(try_from_os_str = Input::try_from)
)]
+    #[clap(value_parser, default_value = "-")]
     input: Input,

-    #[clap(default_value = "-", parse(try_from_os_str = Output::try_fr
om))]
+    #[clap(value_parser, default_value = "-")]
     output: Output,
 }

[max-sixty]

Thanks a lot @aj-bagwell !

We upgraded clio in https://github.com/prql/prql/pull/1003 while keeping clap at 3. We get no messages from cargo check --features clap/deprecated -p prql-compiler.

But upgrading to clap 4.0.8 gives some long errors:

error[E0599]: the method `value_parser` exists for reference `&&&&&&_AutoValueParser<Input>`, but its trait bounds were not satisfied
    --> prql-compiler/src/cli.rs:42:12
     |
42   |     #[clap(value_parser, default_value = "-")]
     |            ^^^^^^^^^^^^ method cannot be called on `&&&&&&_AutoValueParser<Input>` due to unsatisfied trait bounds
     |
    ::: /Users/maximilian/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-4.0.8/src/builder/value_parser.rs:2017:1
     |
2017 | pub struct _AutoValueParser<T>(std::marker::PhantomData<T>);
     | ------------------------------------------------------------ doesn't satisfy `_: clap::builder::via_prelude::_ValueParserViaParse`
     |
    ::: /Users/maximilian/.cargo/registry/src/github.com-1ecc6299db9ec823/clio-0.2.4/src/input.rs:31:1
     |
31   | pub enum Input {
     | --------------
     | |
     | doesn't satisfy `Input: From<&'s std::ffi::OsStr>`
     | doesn't satisfy `Input: From<&'s str>`
     | doesn't satisfy `Input: From<OsString>`
     | doesn't satisfy `Input: From<std::string::String>`
     | doesn't satisfy `Input: FromStr`
     | doesn't satisfy `Input: ValueEnum`
     | doesn't satisfy `Input: ValueParserFactory`
     |
     = note: the following trait bounds were not satisfied:
             `Input: ValueEnum`
             which is required by `&&&&&_AutoValueParser<Input>: clap::builder::via_prelude::_ValueParserViaValueEnum`
             `Input: ValueParserFactory`
             which is required by `&&&&&&_AutoValueParser<Input>: clap::builder::via_prelude::_ValueParserViaFactory`
             `Input: From<OsString>`
             which is required by `&&&&_AutoValueParser<Input>: clap::builder::via_prelude::_ValueParserViaFromOsString`
             `Input: From<&'s std::ffi::OsStr>`
             which is required by `&&&_AutoValueParser<Input>: clap::builder::via_prelude::_ValueParserViaFromOsStr`
             `Input: From<std::string::String>`
             which is required by `&&_AutoValueParser<Input>: clap::builder::via_prelude::_ValueParserViaFromString`
             `Input: From<&'s str>`
             which is required by `&_AutoValueParser<Input>: clap::builder::via_prelude::_ValueParserViaFromStr`
             `Input: FromStr`
             which is required by `_AutoValueParser<Input>: clap::builder::via_prelude::_ValueParserViaParse`
     = note: this error originates in the macro `clap::value_parser` (in Nightly builds, run with -Z macro-backtrace for more info)

Let me know if this is something we're doing wrong!

[aj-bagwell]

The issue is because cargo for reasons that make no sense to me decided that clio should get clap3 and the prql-compiler should get clap4, hence while Input has an impl for ValueParserFactory it is the clap3 ValueParserFactory which is a different type to the clap4 ValueParserFactorywhich it wants.

Some judicious forcing of the versions in the lock file makes it happy, (see this PR).

[max-sixty]

The issue is because cargo for reasons that make no sense to me decided that clio should get clap3 and the prql-compiler should get clap4, hence while Input has an impl for ValueParserFactory it is the clap3 ValueParserFactory which is a different type to the clap4 ValueParserFactorywhich it wants.

😬

Thanks a lot for the PR!