-
Notifications
You must be signed in to change notification settings - Fork 347
feat: allow negative numeric positional arguments #815
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: main
Are you sure you want to change the base?
Conversation
…guments (Fixes apple#31) - Update SplitArguments to classify '-5' and '-3.14' as values, not options - Add unit tests for negative Int and Double positional arguments
@cnaples79 I don't have the chance to look at the code right now, but quick question about "This change gives precedence to numeric literals over short options that would be single-digit flags": Does this completely preclude the ability to use digits as short or old-style-long flags? Or is this only if a signed number could otherwise be accepted in that position? If the latter, could this output a build warning about such clashes when they can occur? |
Thanks for the questions / feedback @rgoldberg !
If you prefer, I can revise to a context-aware approach: only reinterpret "-" as a value when the parser is about to consume a numeric positional and there’s no defined option matching it, which preserves digit flags elsewhere. Workaround users can use today: pass negative numbers with the standard terminator or quoting, e.g. |
@cnaples79 Thanks for the reply. IIUC, quoting a dash followed solely by digits (dash/digits) won't be handled any differently by the shell or by SAP than if it were unquoted. IIRC, there's some setting to make SAP consider dash-prefixed tokens where an option value should have been supplied to be considered an option value or a different flag/option. So that should take care of option values. Maybe there could be a new setting that considers such dash-prefixed tokens to be option values only if the option value type can accept such a token (e.g., a Barring the presence of a prior flag/option terminator, and outside of an option value, I think that dash/digits should by default first match any applicable flags/options, then match any enumerated arguments (like if an enum case had a |
@rgoldberg do you want me to update the PR with this new criteria? |
@cnaples79 I'm not a member of the project. If you agree with my suggestions, by all means make any updates you want. If some updates will take a lot of time, or if you disagree with my suggestions, however, you might want to wait for feedback from @natecook1000 and/or @rauhul, as they know the project better & are the project owners. Thanks for discussing, and for submitting the PR. |
@rgoldberg Ah got it! Yeah of course, I'll wait for feedback from one of the POs. Thanks a bunch for your suggestions! I'm not entirely sure which approach is best yet, will have to do a bit more research. |
@cnaples79 Thanks for working on this! I would have sworn that there was more discussion on #31, but it looks like the notes are fairly brief. Unconditionally treating inputs like |
Summary
Treat '-' tokens as values during argument splitting so that positional arguments of numeric types (Int/Double) can accept negative literals, e.g. or .
Motivation
Currently, inputs like are interpreted as options, causing errors for commands that legitimately expect a negative number as a positional argument (see #31). This change aligns with common CLI expectations where negative numeric literals are valid values.
Implementation
Considerations
Fixes #31