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

Validate argument with regex #1968

Closed
catleeball opened this issue Jun 9, 2020 · 6 comments · Fixed by #2073
Closed

Validate argument with regex #1968

catleeball opened this issue Jun 9, 2020 · 6 comments · Fixed by #2073
Labels
A-validators Area: ArgMatches validation logi E-easy Call for participation: Experience needed to fix: Easy / not much
Milestone

Comments

@catleeball
Copy link

catleeball commented Jun 9, 2020

Describe your use case

It would be nice to have a clap feature for argument validation via a regex. This would be handy and flexible for users, allowing them custom validations without writing a custom validator to do so, and hopefully without requiring too large of overhead for implementation and testing in clap itself.

Describe the solution you'd like

For example, using clap yaml format, if we want an argument whose value wants only digit values 1 through 100:

args:
  - jpg-quality:
    about: Quality of jpg image to write. Must be a number 1-100.
    takes_value: true
    regex: "^[1-9][0-9]?$|^100$"

This could expand to complex regex use cases, like matching if an arg value is a URL:

args:
  - url:
    about: URL to load.
    index: 1
    required: true
    # Regex stolen from: https://stackoverflow.com/a/3809435
    regex: "[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_\+.~#?&//=]*)"

Thanks for this very nice arg parsing library!

@catleeball catleeball changed the title [FR] - Validate argument with regex Validate argument with regex Jun 9, 2020
@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Jun 9, 2020

My first thought was: "This is pretty trivial with normal validators!". And indeed:

lazy_static! {
    static ref RE: Regex = Regex::new("\\d+").unwrap();
}

app.validator(|val| {
    if RE.is_match(val)) {
        Ok(())
    } else {
        Err(format!("value '{}' doesn't match the `\\d+` regex", val))
    }
})

But on the other side, validators cannot be deserialized from YAML, and "matches a regex" does sound common enough to warrant a special case.

I say we might gate it behind regex feature.

Unresolved questions: what about error messages? I have doubts that "value doesn't match regex" is good user-facing message. Perhaps the API should look like:

fn regex_validator("-?\\d+", "expected an integer");

// rendered as
"Invalid value `foo`: expected an integer"

@pksunkara @kbknapp Any thoughts?

@CreepySkeleton CreepySkeleton added A-validators Area: ArgMatches validation logi D: easy E-easy Call for participation: Experience needed to fix: Easy / not much labels Jun 9, 2020
@CreepySkeleton CreepySkeleton added this to the 3.1 milestone Jun 9, 2020
@catleeball
Copy link
Author

Agreed on your point about needing a more helpful error, @CreepySkeleton . I personally like the example function you provided where the user supplies an error string.

@loewenheim
Copy link

How would the error message be written in a YAML file?

@CreepySkeleton
Copy link
Contributor

Just as any other tuple would

regex_validator: 
  - r"[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_\+.~#?&//=]*)"
  - error message

# or if regex and error are short enough
regex_validator: ["-?\\d+", "expected an integer"]

@tmankita
Copy link

Hi,
I would like to pick up this issue if no one is working on it yet.

@epage
Copy link
Member

epage commented May 23, 2022

FYI I am considering removing validator_regex, see #3743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-validators Area: ArgMatches validation logi E-easy Call for participation: Experience needed to fix: Easy / not much
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants