Add new shorthand parser for nested maps #1444
Conversation
This adds a proper parser to the shorthand parser. The parser still maintains the quirky of the old parser where possible to maintain backwards compatibility.
Needed for py3 compat.
| "Expected: '%s', received: '%s' for input:\n" | ||
| "%s\n" | ||
| "%s\n" | ||
| ) % (self.expected, self.actual, self.value, |
There was a problem hiding this comment.
I really like the idea of the '^' pointer. It could be super useful for debugging. How well does it look if the self.value requires more than two lines on the terminal due to wrapping in the terminal? Will the '^' still match up with where the syntax error occurred? I am not sure if I can think of any better representation.
There was a problem hiding this comment.
I don't believe it handles that case. I can update that.
|
The changes look fine. I have just a couple of questions that probably would got have answered with the subsequent PR's that you are sending. |
jamesls
added
incorporating-feedback
and removed
pr:needs-review
| else: | ||
| return self._csv_list() | ||
|
|
||
| def _csv_list(self): |
There was a problem hiding this comment.
This method does a lot, including backtracking the parser. It would be nice for maintainability reasons to have a comment here explaining how it works.
Matches what the previous parser was doing.
Makes the existing structures more succinct.
|
I believe I've incorporated all the feedback here. cc @kyleknap @mtdowling Let me know if I missed anything. |
|
Looks good. Thanks for updating. 🚢 |
* feat(perf): Improve load time of help text * update docker version in isolated.txt file
This adds a new shorthand parser that can handle nested maps via a
{foo=bar}syntax. Note this is not plumbed into the argprocess module yet, it's entirely standalone in this PR.The plan for this is:
There's one point in this PR not implemented that we need to decide. Right now, the explicit list parser
[foo, bar baz]assumes that all the values are strings. If we want to support nested lists or lists of hashes, we will need to require string values that start with[and{to be quoted. There's a comment on line 148 that explains this.cc @kyleknap @mtdowling @rayluo