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

Bug in optflagopt #104

Open
pecastro opened this issue Mar 25, 2021 · 4 comments
Open

Bug in optflagopt #104

pecastro opened this issue Mar 25, 2021 · 4 comments

Comments

@pecastro
Copy link

I've bumped into some weird behaviour in another project whereas the use of an optflagopt option seemed to render unexpected behaviour of the option not being properly parsed.

I decided to come to the source and I extended the mod.rs test to show that something was amiss
pecastro@de5e4db

I've managed to make the test panic when you use an optflagopt in combination with other options.
Initial investigations from debugging the test show that the vals Vec of the Matches struct will have a Vec with Given rather than the option value specified and that is what's causing the return of None and triggering the panic.

self.vals:

[[(0, Val("foo"))], [(1, Val("bar"))], [], [(2, Given)]]
@ghostd
Copy link

ghostd commented Mar 28, 2021

Hi,

It seems to be expected:

getopts/src/tests/mod.rs

Lines 412 to 427 in c11eb65

let long_args = vec!["--test=x".to_string()];
match opts.parse(&long_args) {
Ok(ref m) => {
assert_eq!(m.opt_str("t").unwrap(), "x");
assert_eq!(m.opt_str("test").unwrap(), "x");
}
_ => panic!(),
}
let long_args = vec!["--test".to_string(), "x".to_string()];
match opts.parse(&long_args) {
Ok(ref m) => {
assert_eq!(m.opt_str("t"), None);
assert_eq!(m.opt_str("test"), None);
}
_ => panic!(),
}

@pecastro
Copy link
Author

After digging a bit deeper the issue seems to be mainly to do with the parsing of an optflagopt whereby an -o=foo, -o foo, and --option=foo are parsed correctly whereas the --option foo is leaving foo as free an not consuming it as the argument to the option.

@pecastro
Copy link
Author

@ghostd Yes, I've seen that whilst I was writing a fix and I'm correcting that test which I think it is wrong in my opinion.
The documentation for the method https://docs.rs/getopts/0.2.21/getopts/struct.Options.html#method.optflagopt doesn't specify that when the long option is used any optional parameter should be ignored ...

pecastro added a commit to pecastro/getopts that referenced this issue Mar 28, 2021
When the optflagopt long version is used the parser is ignoring the argument.
@pecastro
Copy link
Author

pecastro commented Mar 28, 2021

Upon further attentive reading I did spot under the Parsing section of the Linux getopt user command https://man7.org/linux/man-pages/man1/getopt.1.html on the second to last paragraph the rules of engagement regarding the handling of optional arguments.
What tripped me was the absence in this documentation of any similar indication regarding the behaviour.
The https://man7.org/linux/man-pages/man3/getopt.3.html library does not state any such behaviour.

I've close the associated MR for now and I'll close this afterwards if no other insightful comments materialize ...

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

No branches or pull requests

2 participants