Clap: suboptimal flag suggestion

[BurntSushi]

Rust Version

rustc 1.17.0 (56124baa9 2017-04-24)

Affected Version of clap

2.26.2

Expected Behavior Summary

CLI error message suggests --files-without-match

Actual Behavior Summary

CLI error message suggests --files-with-matches

Steps to Reproduce the issue

$ cargo new ripgrep-616 --bin
$ $EDITOR Cargo.toml
... add `clap` to dependencies
$ cat > src/main.rs <<EOF
extern crate clap;

use clap::{App, Arg};

fn main() {
    let app = App::new("ripgrep-616")
        .arg(Arg::with_name("files-with-matches").long("files-with-matches"))
        .arg(Arg::with_name("files-without-match").long("files-without-match"));
    app.get_matches();
}
EOF
$ cargo run -- --files-without-matches
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target/debug/ripgrep-616 --files-without-matches`
error: Found argument '--files-without-matches' which wasn't expected, or isn't valid in this context
        Did you mean --files-with-matches?

USAGE:
    ripgrep-616 --files-with-matches

For more information try --help

Sample Code or Link to Sample Code

See above.

Debug output

cargo run -- --files-without-matches
   Compiling clap v2.26.2
   Compiling ripgrep-616 v0.1.0 (file:///tmp/ripgrep-616)
    Finished dev [unoptimized + debuginfo] target(s) in 16.63 secs
     Running `target/debug/ripgrep-616 --files-without-matches`
DEBUG:clap:Parser::propogate_settings: self=ripgrep-616, g_settings=AppFlags(
    NEEDS_LONG_HELP | NEEDS_LONG_VERSION | NEEDS_SC_HELP | UTF8_NONE | COLOR_AUTO
)
DEBUG:clap:Parser::get_matches_with;
DEBUG:clap:Parser::create_help_and_version;
DEBUG:clap:Parser::create_help_and_version: Building --help
DEBUG:clap:Parser::create_help_and_version: Building --version
DEBUG:clap:Parser::get_matches_with: Begin parsing '"--files-without-matches"' ([45, 45, 102, 105, 108, 101, 115, 45, 119, 105, 116, 104, 111, 117, 116, 45, 109, 97, 116, 99, 104, 101, 115])
DEBUG:clap:Parser::is_new_arg: arg="--files-without-matches", Needs Val of=NotFound
DEBUG:clap:Parser::is_new_arg: Arg::allow_leading_hyphen(false)
DEBUG:clap:Parser::is_new_arg: -- found
DEBUG:clap:Parser::is_new_arg: starts_new_arg=true
DEBUG:clap:Parser::possible_subcommand: arg="--files-without-matches"
DEBUG:clap:Parser::get_matches_with: possible_sc=false, sc=None
DEBUG:clap:Parser::parse_long_arg;
DEBUG:clap:Parser::parse_long_arg: Does it contain '='...No
DEBUG:clap:Parser::parse_long_arg: Didn't match anything
DEBUG:clap:Parser::groups_for_arg: name=files-with-matches
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:usage::create_usage_with_title;
DEBUG:clap:usage::create_usage_no_title;
DEBUG:clap:usage::smart_usage;
DEBUG:clap:usage::get_required_usage_from: reqs=["files-with-matches"], extra=None
DEBUG:clap:usage::get_required_usage_from: after init desc_reqs=[]
DEBUG:clap:usage::get_required_usage_from: no more children
DEBUG:clap:usage::get_required_usage_from: final desc_reqs=["files-with-matches"]
DEBUG:clap:usage::get_required_usage_from: args_in_groups=[]
DEBUG:clap:usage::get_required_usage_from:iter:files-with-matches:
DEBUG:clap:Parser::color;
DEBUG:clap:Parser::color: Color setting...Auto
DEBUG:clap:is_a_tty: stderr=true
DEBUG:clap:Colorizer::error;
DEBUG:clap:Colorizer::warning;
DEBUG:clap:Colorizer::good;
error: Found argument '--files-without-matches' which wasn't expected, or isn't valid in this context
        Did you mean --files-with-matches?

USAGE:
    ripgrep-616 --files-with-matches

For more information try --help

Other

This bug was reported to ripgrep: https://github.com/BurntSushi/ripgrep/issues/616

Note that I don't necessarily think this can or should be fixed. In particular, these sorts of algorithms often involve heuristics, and making one case better may make other cases worse. Regardless, I wanted to shuffle this off to you in case there was something here.

[kbknapp]

Thanks for pinging me and taking the time to file this! I've been swamped lately and haven't viewed my notifications in quite some time :smile:

I'm pretty sure this is due to the heuristics used in strsim which is actually quite tweakable. I'm currently using a moderately arbitrary configuration, so it almost certainly can be improved. It's also code from two years ago (according to git blame), and strsim has changed and added features since then.

In this particular case if we discounted all matching characters in sequence, and only "compared" the remainder I'm sure it'd be more accurate. But I don't know if that would change other cases without a bit of testing.

So I'll leave this open in case someone wants to take a stab at it. Although, with the issues I've got in the backlog it'll be a low priority issue for me.

[pksunkara]

@pickfire Since you worked on https://github.com/clap-rs/clap/pull/1710, do you think you can make a PR for this after testing that this is still an issue?

[pickfire]

@pksunkara Thanks for pinging me on this old issue, I am surprised you still remembered it. I will check and add it to the tests.

[pickfire]

@pksunkara No, that PR does not affect this. The output is still the same, probably.

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/clap --files-without-matches`
error: Found argument '--files-without-matches' which wasn't expected, or isn't valid in this context
        Did you mean --files-without-match?
If you tried to supply `--files-without-matches` as a PATTERN use `-- --files-without-matches`

USAGE:
    clap --files-without-match

For more information try --help

But yes, the issue @BurntSushi is fixed. It recommends --files-without-match now. Should I still write a test for this?

[pksunkara]

Yes, please add a test. If you see here, the current test works with only 1 option.