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

configuration: improve getopt_long error reporting #427

Merged
merged 1 commit into from
Mar 17, 2024

Conversation

perillo
Copy link
Contributor

@perillo perillo commented Mar 16, 2024

Currently, in case of an unrecognized option we get two error messages. The first is printed by getopt_long and the second is printed by kcov error handler.

An additional issue is that the kcov error handler prints optopt, but this value is 0 (NULL character) in this case.

Improve the error handler by using optind to get the offending option. This is done in a separate switch case, instead of the default case. Disable getopt error messages by setting opterr to 0.

Configure getopt_long to report a missing argument using a different error code, so that this can be handled in a separate switch case. Note however that this error is only reported when the option is the last argument.

Update the basic.TooFewArguments and basic.WrongArguments test cases.

TODO

Print useful error message in case of an invalid option argument. This PR will not implement it.

Currently, in case of an unrecognized option we get two error messages.
The first is printed by getopt_long and the second is printed by kcov
error handler.

An additional issue is that the kcov error handler prints optopt, but
this value is 0 (NULL character) in this case.

Improve the error handler by using optind to get the offending option.
This is done in a separate switch case, instead of the default case.
Disable getopt error messages by setting opterr to 0.

Configure getopt_long to report a missing argument using a different
error code, so that this can be handled in a separate switch case.
Note however that this error is only reported when the option is the
last argument.

Update the basic.TooFewArguments and basic.WrongArguments test cases.
@perillo
Copy link
Contributor Author

perillo commented Mar 16, 2024

The new test should probably be skipped, since it is preventing full testing.

For the next PRs I plan to improve the Python code running the test suite, so the tests should work without issues.

UPDATE
Fixed the bug in #428.

@SimonKagstrom
Copy link
Owner

That's a nice improvement of the option parsing, thanks a lot!

@SimonKagstrom SimonKagstrom merged commit 49635e7 into SimonKagstrom:master Mar 17, 2024
4 of 8 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.

2 participants