Parse number expansions in args #5973
Conversation
There was a problem hiding this comment.
Nothing interesting in here, just rote rewrites from either ... ... into case statements.
4853fc5 to
684a9e7
Compare
| When running a main function in `ucm` a numeric argument is replaced by the potential last result of a find command: | ||
|
|
||
| ``` unison | ||
| The workaround for this is to quote the numeric argument instead. |
There was a problem hiding this comment.
@aryairani
The changes to behaviour in this file are worth looking over.
There was a problem hiding this comment.
Thanks for calling it out.
It is kind of weird (but not unprecedented) to have to quote the numbers to get normal strings, but the alternative (to have a special syntax for numbered args, which then we still have to be able escape to not expand) seems very hairy, so this seems good for now.
There was a problem hiding this comment.
So, not for this PR, but how hard would it be to let the commands decide whether they'll allow numbered args or just take them unsubstituted? This question arose when I realized we're not trying to support passing an expanded numbered arg to run anyway.
There was a problem hiding this comment.
We can't detect that at parsing time very easily, but we can at argument-matching time;
So the weird part would be, after we've already parsed an argument as a number, we then render it back to a string in order to pass it as an arg.
Right now I think that's fine because I believe each numeric range only has a single string representation, but if that ever becomes not 1-to-1 then things could get weird; e.g. you pass 1.0-2 as an argument to run, but your program actually runs with the string 1-2 because it round-tripped through the parser haha (again, not an issue at the moment though)
Overview
Number expansion was done relatively ad-hoc, and it was impossible to pass a number as an argument to
runsince it would always attempt to expand it.With #5968 the parser is more structured, so now we can actually parse numbered ranges correctly.
1-99or w/e, so I added support for open ranges, e.g.1-or-10Implementation notes
Interesting/controversial decisions
I can change or remove the support for open ranges if desired.
I changed the expected behaviour of the transcript for #2805 ;
Personally I think it's quite confusing if the way a command line is parsed is dependent on the previous command.
I changed it to no longer change behaviour based on the numbered args state; but rather if you want to pass a numeric argument you can always quote it to get the desired effect regardless of the state.
Test coverage
Transcripts
Loose ends
We could support fully open ranges like
-, but I left that off for now because-is a valid symbol on its own.