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

Clearer error messages for ArgGroups #872

Open
scottjasso opened this issue Nov 20, 2019 · 1 comment
Open

Clearer error messages for ArgGroups #872

scottjasso opened this issue Nov 20, 2019 · 1 comment
Labels
theme: parser An issue or change related to the parser

Comments

@scottjasso
Copy link

scottjasso commented Nov 20, 2019

See #870.

  • If num groups > max(multiplicity) (whether the args are optional or required), it's currently:

    Error: expected only one match but got (--group=<name> [--opt1=<opt>])={--group=foo --opt1=1} and (--group=<name> [--opt1=<opt>])={--opt1=1}

    I like that this shows you the matched groups, but (1) it always says "expected only one match", even if the minimum multiplicity > 1, and (2) it seems unnecessary to repeat the group spec.

    If num groups < min(multiplicity), it's currently:

    Error: Group: (--group=<name> [--opt1=<opt>]) must be specified 2 times but was matched 1 times

    For consistency, I think both cases should be something like:

    Error: (--group=<name> [--opt1=<opt>]) must be specified [X / at least X / at most X] time(s) but was matched 2 times: {--group=foo --opt1=1}, {--opt1=2}

    This makes it clear how the groups are, well, grouped :)

  • If missing a required option for a group, it's currently:

    Error: Missing required argument(s): --group=<name>

    IMO something like

    Error: --group=<name> is required in (--group=<name> [--opt1=<opt>]), got: {--opt1=1}

    This clarifies that --group=<name> is only needed when specifying --opt1. This also applies for mutually dependent args:

    Error: --req-arg1=<foo>, --req-arg2=<bar> are all required in (--req-arg1=<foo>, --req-arg2=<bar> [--opt1=<opt>]), got: {--req-arg1=foo}

Also highly relevant to #871 -- there are some questions raised there that may shape how the errors are printed / validation is done.

@deining
Copy link
Contributor

deining commented Nov 20, 2019

Please note the previous discussion on this topic (#744) which led to a code change to address the issue. I agree that there is still room for improvement, though.

@remkop remkop added the theme: parser An issue or change related to the parser label Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: parser An issue or change related to the parser
Projects
None yet
Development

No branches or pull requests

3 participants