Warnings from clap 3.2

[max-sixty]

Following on from #1, we're getting some warnings since upgrading to Clap 3.2 in https://github.com/prql/prql/pull/597

warning: use of deprecated variant `clap::ArgAction::StoreValue`: Replaced with `ArgAction::Set` or `ArgAction::Append`
  --> prql-compiler/src/cli.rs:48:37
   |
48 |     #[clap(default_value="-", parse(try_from_os_str = TryFrom::try_from))]
   |                                     ^^^^^^^^^^^^^^^
   |
   = note: `#[warn(deprecated)]` on by default

warning: use of deprecated variant `clap::ArgAction::StoreValue`: Replaced with `ArgAction::Set` or `ArgAction::Append`
  --> prql-compiler/src/cli.rs:51:52
   |
51 |     #[clap(short, long, default_value = "-", parse(try_from_os_str = Output::try_from))]
   |                                                    ^^^^^^^^^^^^^^^

Possibly we're doing something wrong (I actually don't know this well, the esteemed @kwigley generously implemented this).

Thank you!

[kwigley]

(hey, @max-sixty!) Ah, looks like some more clap api changes. Looking at https://github.com/clap-rs/clap/blob/master/CHANGELOG.md#320---2022-06-13

• For default parsers (no parse attribute), deprecation warnings can be silenced by opting into the new behavior by adding either #[clap(action)] or #[clap(value_parser)] (ie requesting the default behavior for these attributes). Alternatively, the unstable-v4 feature changes the default away from parse to action/value_parser.
• For #[clap(parse(from_flag))] replaced with #[clap(action = ArgAction::SetTrue)] (#3794)
• For #[clap(parse(from_occurrences))] replaced with #[clap(action = ArgAction::Count)] though the field's type must be u8 (#3794)
• For #[clap(parse(from_os_str)] for PathBuf, replace it with #[clap(value_parser)] (as mentioned earlier this will call value_parser!(PathBuf) which will auto-select the right ValueParser automatically).
• For #[clap(parse(try_from_str = ...)], replace it with #[clap(value_parser = ...)]
• For most other cases, a type implementing TypedValueParser will be needed and specify it with #[clap(value_parser = ...)]

Thanks for the nifty little library by the way! If you are interested in maintaining compatibility with these clap changes, I'm happy to suggest a patch.

[kwigley]

@max-sixty it also looks like these deprecation warnings are turned off in the latest version of clap

• https://github.com/clap-rs/clap/blob/master/CHANGELOG.md#fixes-1

Cargo checks are passing for https://github.com/prql/prql/pull/598 which uses this latest version 👍

[max-sixty]

Thanks a lot @kwigley !

[aj-bagwell]

@kwigley I was looking forward to this clap update as it fixes the issue I had with try_from being called twice. I am looking to support the #[clap(value_parser)] syntax.

The only drawback is that to work with value_parser the type needs to be Clone, Send, and Sync. File is not Clone and the Http clients (if that feature is enabled) are not Sync.

My current plan is to implement clone by calling File::try_clone and panicing if it does not work. I don't like the panic and I don't like that the clones share a handle and therefore file position but I don't have a better answer. However using the derive syntax clap never calls clone, so it is not really an issue.

[aj-bagwell]

I have updated clio with a clap-parse feature to work with value_parser it is in version 0.2.2