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

Support custom suggested fix messages for specific flags #4706

Closed
2 tasks done
weihanglo opened this issue Feb 13, 2023 · 10 comments · Fixed by #5075
Closed
2 tasks done

Support custom suggested fix messages for specific flags #4706

weihanglo opened this issue Feb 13, 2023 · 10 comments · Fixed by #5075
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing

Comments

@weihanglo
Copy link

weihanglo commented Feb 13, 2023

Please complete the following tasks

Clap Version

4.1.4

Describe your use case

Clap has a good fix suggestion mechanism when a user types something wrong. It seems to be based on Jaro distance.

A developer sometimes wants to provide an additional context on a specific situation to users. In order to do so, clap needs to support things like “when a user passes a non-existent flag/command/value/whatever, suggest a flag with this custom message instead.”

Describe the solution you'd like

There are lots of different ways and layers to support this feature.

I'll try to dump only one silly idea here: Clap seems to have several kinds of suggested fix. I don't have any knowledge of clap internals, but feel like at least Arg and Command can have a method, say suggests, accepting a flag name and an alternate message, such as

 Arg::new("directory")
     .help("Change to DIRECTORY before doing anything")
     .short('C')
     .value_name("DIRECTORY")
+   .suggests("--cwd", "long flag --cwd is not yet supported. Please use `-C` instead")

Not a good idea but you've got the gist.

Alternatives, if applicable

Do it manually on application-side. Like

They are not really in the same situation of this feature requests, as Cargo keeps a stable flag way longer than usual. However, if Cargo decide to remove those subcommand/flags, it may utilize this feature to provide good suggestions.

Additional Context

This idea was originally from rust-lang/cargo#11702

@weihanglo weihanglo added the C-enhancement Category: Raise on the bar on expectations label Feb 13, 2023
@epage
Copy link
Member

epage commented Feb 13, 2023

In rust-lang/cargo#11702, my original idea was for this to allow the user to provide custom suggestions, similar to the automatic ones.

This would look like

 Arg::new("directory")
     .help("Change to DIRECTORY before doing anything")
     .short('C')
     .value_name("DIRECTORY")
-   .suggests("--cwd", "long flag --cwd is not yet supported. Please use `-C` instead")
+   .suggest(["--cwd", "--directory", "--cd"])

with the error message:

$ cargo --cwd ../foo
error: unexpected argument '--cwd' found

  note: argument '-C' exists

Usage: cargo [COMMAND]

For more information, try '--help'.

@epage
Copy link
Member

epage commented Feb 13, 2023

Aside: In a way, this reminds me of #3321, especially with how this issue is written with providing custom error messages

@epage
Copy link
Member

epage commented Feb 13, 2023

Implementation note: we'll need to be careful that we can discover these for suggestions without

  • Being used for inferring
  • Being provided as dynamic suggestions so long as valid arguments are suggested

I wonder if we could use this in completions so we complete --directory as -C?

@epage
Copy link
Member

epage commented Aug 11, 2023

What if we instead supported this as

 Arg::new("directory")
     .help("Change to DIRECTORY before doing anything")
     .short('C')
     .value_name("DIRECTORY"),
 Arg::new("cwd")
     .long('cwd')
     .value_name("DIRECTORY")
     .hide(true)
     .action(ArgAction::Unsupported("long flag --cwd is not yet supported. Please use `-C` instead".into()))
  • debug_asserts will ensure ArgAction::Unsupported is used with `hide(true)
  • If completions show hidden arguments, then they'll need to hide these

@weihanglo
Copy link
Author

That looks nicer and more consistent! What is the behavior in your mind when receiving such an arg, bail out immediately?

@epage
Copy link
Member

epage commented Aug 11, 2023

I'm leaning towards showing an unexpected argument error with the message inside of the tip

@weihanglo
Copy link
Author

Things Cargo also wants:

  • Suggesting a new subcommand for a deprecated subcommand.
  • Instead of erroring out, some flags just need a deprecation warning and continue working.

I guess for the first one, cargo needs to match the Result by itself. For the latter, ArgAction doesn't apply to the situation.

Also, should we talk about custom validation error message as a whole? I don't really understand clap internals so they might be different stuff.

@epage
Copy link
Member

epage commented Aug 11, 2023

Native deprecation support is being tracked in #3321

Also, should we talk about custom validation error message as a whole? I don't really understand clap internals so they might be different stuff.

Arg::value_parser allows validating an argument and converting it into a application-domain type

Is that what you are looking for?

@weihanglo
Copy link
Author

I see.
Yep. value_parser might work. Will look into how to integrate.

@epage
Copy link
Member

epage commented Aug 16, 2023

Could people look over #5075 to see if it works for their needs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants