spectre.console
spectre.console copied to clipboard
Cannot specify negative number as command option value
As of version 0.40.0, it doesn't appear possible to specify a negative number as a command option.
Assuming I define a command that takes a decimal value as a command option, and that I want to pass a negative number value, neither of the following works:
mycommand -a -1.50
mycommand -a "-1.50"
Both attempts fail with this error Not a valid name for a short option. It appears that the command option parsing logic thinks that the -1.50 is supposed to be a command option.
I would expect that the quotes at least would effectively escape the numeric value. Is this a bug, or am I missing something?
Thanks.
You could try mycommand -a=-1.50
Great - that workaround was successful.
I'm considering taking a stab at this if it's open for contributions.
I was thinking about changing CommandTreeTokenizer.ScanShortOptions to turn a short option that does not start with a letter into a token of type String instead of throwing CommandParseException.InvalidShortOptionName (at least for numeric strings following the dash), but I'm not sure if there are harmful consequences to that. It'll break the InvalidShortOptionName test for certain, but I'm not sure that's relevant; perhaps checking that at the tokenizer level feels a bit unnecessary, and a check somewhere higher up the chain where the contents can actually compared to the expected option set might be more appropriate. What are your thoughts on this?
Also, any ideas on what would be the best approach to unit testing such a change? Haven't looked very deeply into the unit test base yet, but didn't immediately come across an example of a test that would test against the parsed Settings object.
@patriksvensson I'm not sure you ever got notified of my previous question, so another ping here. Not in a hurry from my side though, just making sure you've seen it.
@jouniheikniemi As far as we know, no one is currently working on this. So, sure, take your stab.
@nils-a Actually, when I was looking at this in more detail, I started wondering if this is actually the right thing to do, and if so, how to do it without painting oneself into a future corner. In other words, given how Spectre.Cli used to say it is "extremely opinionated", so what's its opinion here?
If the library starts tokenizing things looking like negative numbers (-1 etc.) as strings instead of options, what happens when somebody wants to have a parameterless option like -1? That has happened; git diff perhaps being one of the more known examples. POSIX specifically states that option names are single alphanumerical characters.
Now, obviously POSIX compliance may certainly not be the opinion Spectre.Console has, but if (and when) it's not, what is? The CLI documentation mentions a couple of parsing models, but it doesn't mention the ability to attach an equal sign to a short option.
Taking POSIX/GNU CLI spec as a starting point, the switch syntaxes could be like
-a (a valueless short switch) --automatic (a valueless long switch) -abc (three valueless short switches) -a foo (a short switch with a value)
But then,
-afoo should also be supported (equal to -a foo) --age=21 should be the approach for writing long arguments (instead of just --age 21)
Fiddling with the parsing of the minus sign easily gets in the way of future needs, so I thought this might be a good time to ask what's in your mind. Obviously, POSIX command lines are not meant to be parsed without the parser knowing the option set in beforehand (otherwise it would be impossible to know if -xbar is intended to be parsed as -x -b -a -r or -x bar), so going that way would require a bit of a different take on the parser.
Do you have a master plan here? I personally prefer POSIX/GNU parsing in everything I do, but that's obviously not where we stand now. If the "extreme opinion" is specifically something else, what is it? Understanding that would probably help a contributor evolve the parser code in the right direction. Thanks :-)
@jouniheikniemi I'll try to answer all your questions.
[...] Spectre.Cli used to say it is "extremely opinionated", so what's its opinion here?
That would be @patriksvensson opinion of things. I think there's no fully written down version of it 😄
Do you have a master plan here?
Again, nothing that's written down or anything.
Currently, options follow the following guidelines:
- option names can only be letters or digits (
char.IsLetterOrDigit) - no option can start with a digit
- each option can have one short name (starting with
-, followed by exactly one character) or one long name (starting with--) or both. - if options have a value, that value needs to be passed after the option, separated by either a space (
) or equal (=) sign. (I think passing a colon (:) is also possible)
(I hope this is all of them, and I did not forget anything.)
I personally prefer POSIX/GNU parsing in everything I do
While I do feel that implementing an existing standard is always a good thing, I also feel that implementing POSIX is a bit too much, here. (And, I personally dislike -afoo for -a foo and -xvjf for -x -v -j -f.)
So, to summarize the above, currently -1 and --11 are not valid options, and they were planned as such. At the same time, I see the inability to pass -1 as an argument to an option using e.g. --foo -1 instead of --foo=-1 as a bug.
While I do feel that implementing an existing standard is always a good thing, I also feel that implementing POSIX is a bit too much, here.
Oh I'm certainly not ready to write a full POSIX compliance PR either, now 😅
Thanks! My next week is busyish, but I'll take a look once I have a moment.
Just to clarify.
-a (a valueless short switch) --automatic (a valueless long switch) -abc (three valueless short switches) -a foo (a short switch with a value) --age=21
All these things are supported by the Spectre.Console parser.
The "opinionated" part comes from the fact that when I wrote the parser, I wanted to make it somewhat POSIX compliant, but also make it easier for people coming from Windows to use it. For example, /foo:3 is supported as well.
Some things like you mention above, such as -xbar which could be interepreted as -x bar OR -x -b -a -r, I chose not to support, since I personally think it's a bit confusing.
@nils-a @patriksvensson I have now (finally) implemented this per the original idea (i.e. allow negative numbers to be parsed as option values). The implementation is actually a bit broader than that, and also applies to positional arguments. Previously if you had a command with syntax "temperature
I have one additional question for you: Would you prefer an implementation per the original issue scope (i.e. only permit negative numbers), or should we just allow anything that starts with a dash and a digit to be parsed as a string and leave type validation up to the parser? There are certain scenarios where this might be relevant; for example, I have one application that takes ranges such as "-5..5", which is not a negative number but would be a valid string, though one the current Cli tokenizer would not accept.
I wrote the extra few lines of validation required for "negative numbers only", but I'm happy to throw it out if you want to permit anything that could be a number to be tokenized as a string. IMO, it would probably be better to be more lenient at the tokenizer level, since somebody just might have a value representation that starts like a negative number but isn't one. Thoughts?
@jouniheikniemi good thinking. I like the -5..5 possibility. 👍
OK, permitted anything starting with a dash and a number. PR #732 is waiting for review.
I'm closing this issue as it has been fully addressed by https://github.com/spectreconsole/spectre.console/pull/1048, which has been merged into the main branch