Add loose parse. #533
Add loose parse. #533
Conversation
src/loose/state.js
Outdated
|
|
||
| parse() { | ||
| let node = this.options.program || this.startNode() | ||
| this.next() |
There was a problem hiding this comment.
I noticed in the default parser the call is this.nextToken() but I saw that parse_dammit uses this.next().
I'm not sure which is best.
There was a problem hiding this comment.
Hmm, I believe they're different in parser/loose parser, but not completely sure as almost didn't touch the loose one. @marijnh should know better.
There was a problem hiding this comment.
Yes, the loose parse class has a next method, which wraps the regular tokenizer with some error-correcting magic.
I think you should just move the body from parse_dammit into this method (which I guess you did) and have parse_dammit call it, to avoid duplication.
There was a problem hiding this comment.
I noticed some other differences between loose and strict. The default Parser constructor accepts options, input but the LooseParser accepts input, options.
| @@ -41,9 +41,7 @@ defaultOptions.tabSize = 4 | |||
|
|
|||
| // eslint-disable-next-line camelcase | |||
| export function parse_dammit(input, options) { | |||
| let p = new LooseParser(input, options) | |||
| p.next() | |||
| return p.parseTopLevel() | |||
There was a problem hiding this comment.
Interesting that before it didn't pass a node to parseTopLevel but to align with the strict parse I went ahead and did that too. I also used const instead of let (though the Parser equiv should use const too).
There was a problem hiding this comment.
parseTopLevel doesn't take an argument. To clarify, these are completely different classes. A loose parser has a regular parser, but doesn't inherit from it.
There was a problem hiding this comment.
Updated to remove the node use. I think it'd be beneficial if their APIs aligned a bit more. It seems kind of odd for them to be different in little ways here and there.
There was a problem hiding this comment.
They often do different things, and have different constraints. I don't think aligning methods is much of priority.
There was a problem hiding this comment.
You do see how having two methods named parseTopLevel that do two different things, with different argument signatures, is kind of confusing though, right?
|
Updated. |
|
Great, merged as 9dbcf60 |
Addresses #532. This PR is a start. It could use a test too.