-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Support short option bundling #16563
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
base: master
Are you sure you want to change the base?
Conversation
4eaf449 to
7ddb07f
Compare
|
Does the |
|
Related to #10981 |
|
Refs #11537 |
I would not have used it if not for the fact there is already another |
|
It would be nice if |
|
Agreed, but technically a breaking change as it currently will pass the rest of the characters in the sequence through as the value which someone could be depending on. I went for putting it behind a flag so as to not break anyone, but the default could be swapped in a future release if we wanted. require "option_parser"
# current behaviour:
OptionParser.parse(%w(-vvvv)) do |parser|
parser.on("-v", "") { |input| puts input } # logs "vvv", the remainder of characters after first v
end
# with bundling: true set:
level = 0
OptionParser.parse(%w(-vvvv), bundling: true) do |parser|
parser.on("-v", "") { |input| level += 1 } # increments level four times
end |
|
I don't think we need either
How does your implementation behave with the following case?
require "option_parser"
OptionParser.parse(%w[-aeb], gnu_strict_bundling: true) do |parser|
parser.on("-a", "") { p! a }
parser.on("-b", "") { p! b }
parser.on("-e VALUE", "") { |e| p! e }
end |
|
It consumes |
7ddb07f to
4a0a48a
Compare
|
I think the bundling limitation doesn't come from nowhere. If a short option takes a value then it's confusing to have it appear inside a bundle: is it followed by a value or by another short option? IMO it makes sense to fail to parse |
|
I don't disagree that a short option which takes a parameter being used in the bundle consumes the rest of the string is confusing, but that is how getopt is specified. Any option with a short flag can be bundled and, if it takes a value, it will consume the rest of the characters. We could deviate on that particular note, or add another option to fail in such cases. Not too sure how to proceed there. 🤔 |
|
Are you sure? It's from the Wikipedia page,
It means that glibc EDIT: I found the guidelines in POSIX that state the same:
I'll verify how it is in practice, but need to work out the C API 😅 EDIT 2: I'm not requesting the change, let's investigate the usual behavior and discuss before we decide how to proceed. |
|
The getopt reference example shows |
|
You're right. I tested and it indeed works like that. I guess the whole |
|
Yeah. A bit weird ergonomics, but it is what it is. 🤷🏻 |
4a0a48a to
70f88e8
Compare
This enables short option bundling such as with `-rf` which
will with this option trigger separate `-r` and `-f` handlers.
```
require "option_parser"
removed = false
forced = false
OptionParser.parse(%w(-rf), bundling: true) do |parser|
parser.on("-r", "") { removed = true }
parser.on("-f", "") { forced = true }
end
{removed, forced} # => {true, true}
```
70f88e8 to
e2279d2
Compare
| # parser.on("-f", "") { forced = true } | ||
| # end | ||
| # | ||
| # {removed, forced} # => {true, true} |
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.
Polish:
| # {removed, forced} # => {true, true} | |
| # p removed # => true | |
| # p forced # => true |
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.
p is not necessary :)
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.
Well, you'd see nothing if you were to run the example.
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.
# => denotes return value, not the output printed
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.
Ah, yeah, the return value is what I had in mind with that. Is explicit logging in examples a more standard pattern we try to follow?
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.
AFAIK, examples tend to focus on return values and not the output - unless there's a reason to do so, that is.
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.
@Qard No, we should have a check around stdlib for that. It's probably just me.
This enables short option bundling such as with
-rfwhich will with this option trigger separate-rand-fhandlers.Fixes #10981