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

Improved clarity in RequiredError #1029

Merged
merged 2 commits into from
Apr 7, 2024

Conversation

sifferman
Copy link
Contributor

Hello. Thank you for this tool.

I found a few small typos in the RequiredError class.

This is an error message I received:

Requires at most 2 options be used and 3were given from [filename,--get-size,--get-size2]

This PR changes the error message to

Requires at most 2 options be used but 3 were given from [filename,--get-size,--get-size2]

Happy to make changes as needed!

Source

    std::string filename;
    bool get_size = false;

    CLI::App app{"Program to read a file specified as command-line argument"};

    app.add_option("filename", filename, "File to read")->required();
    app.add_flag("--get-size", get_size, "Print the size of the file");
    app.add_flag("--get-size2", get_size, "Print the size of the file2");

    app.require_option(1, 2); // Enforce only one flag can be input

    CLI11_PARSE(app, argc, argv);

Further Idea

Also, another idea I had was to remove required options from this error message. In this example, "filename" is required, so a better error message would be

Requires at most 1 options be used but 2 were given from [--get-size,--get-size2]

I'm happy to look into this if you feel it would be valuable

@henryiii
Copy link
Collaborator

henryiii commented Apr 7, 2024

Sure, sounds like a reasonable further cleanup; thanks for this!

@henryiii henryiii merged commit 8bf340d into CLIUtils:main Apr 7, 2024
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants