-
-
Notifications
You must be signed in to change notification settings - Fork 175
Optarg: handle "--" case #2583
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
Optarg: handle "--" case #2583
Conversation
4cc5a94 to
8e26302
Compare
|
This looks great, thank you! Just a minor comment about the tests |
|
@andychu I think the comment is missing |
spec/builtin-getopts.test.sh
Outdated
| a | ||
| ## END | ||
|
|
||
| #### getopts sets name to '?' when encountering '--' #2579 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can combine these 3 test cases
- it processes the flag -a before --
- it sets name to
? - it sets OPTIND
and then the last test case tests with args -c operand
So it could be 2 cases: without args after --, and with args
And in each case it checks 3 things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had one test initially, but then I found out zsh failed it. So then I split it up to make it more clear what exactly zsh does differently. But then I found out that these tests don't run zsh, so it doesn't show up anyway. I tried adding zsh but it was failing a lot of test cases so I ended up leaving it out. Typing it out I guess there is indeed no more reason for these to be split up, so I'll merge it back into one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I split it up into 2 tests, where the second test is more focussed on leaving the remaining args as operands
|
Oh sorry! I didn't "publish" my code review |
8e26302 to
0f5a1b9
Compare
0f5a1b9 to
d2e36b0
Compare
|
Thanks! |
Fixes #2579
POSIX specifies specific behavior when "--" is passed as an argument to getopts, but osh was not handling this case:
"The first -- argument that is not an option-argument should be accepted as a delimiter indicating the end of options. Any following arguments should be treated as operands, even if they begin with the '-' character."
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html
Furthermore getopts should return
?when it is processing"--"according to