Add structure to UCM command args #5480
Draft
+1,059
−818
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This changes the UCM command parser quite a bit.
The args now have to match the parameter definitions
First, this renames some things to make the distinction between parameters and arguments provided to those parameters clearer. This is most apparent in the
InputPattern.args
toInputPattern.params
change.Previously, there were a lot of commands which omitted or had incorrect parameter definitions. This prevented completion from working for those, and just allowed things to get a bit out of sync. This is no longer possible. They now have to match.
But an
InputPattern
’sParameters
are now used to determine whether an argument is “structured” or not. Structured args have numbered arg substitution applied, while unstructured args do not. So the arguments torun
are unstructured, meaning numbers can be passed as arguments now.Also, the arguments are lexed with Unison’s lexer, which (among other things) handles quoted strings consistently. This now allows for file paths containing spaces, and arguments to other functions (
run
,create.author
, etc. now consistently allow quoted strings)Fixes #2805 and #5193.
Implementation notes
It creates a more rigid
Parameters
structure is then folded alongside a list of arguments. As each argument is processed, it is optionally (based on the individualParameter
) passed through numeric expansion. That can cause an argument to be replaced with multiple arguments (because of ranges), which then cascade to the later parameters.The result of the fold is a pair of remaining parameters and the expanded arguments that have been processed. If there are unsatisfied required parameters, then it its handed off to fuzzy completion.
Interesting/controversial decisions
This uses the existing Unison lexer. It seems like a better approach might be to use some of the lexing functions in a new (simpler) lexer that also supports lexemes for projects, branches, etc.
Test coverage
The transcripts check a lot of this.
Loose ends
This is still a WIP. It mostly works, but there are a couple remaining rough edges, and the code needs to be cleaned up.
Right now, I think it requires quoting strings a bit more often than would be ideal (basically, anything that doesn’t form a valid Unison lexeme, so
"@project/branch"
, versions like"1.2.3"
, and a couple other cases). The error reporting needs to be improved as well.I mentioned having a more specific lexer for this as one bit of further work (which would eliminate the extra quoting). Alternatively, we could lex incrementally – with each
ParameterType
specifying whether the next argument should be lexed or just taken to the next space.Another good change would be to move the argument handling functions from the individual command parsers to the
ParameterType
structure.