feat: include paren tokens around scope #37
Conversation
There was a problem hiding this comment.
A couple notes, this looks like a great start to me though 👍
- we might not need to add new tests, but will need to update the snapshots and make sure they have the appropriate new nodes.
- let's perhaps update the grammar in the README to replace
(and)with their expansion?
| let s = scope(scanner) | ||
| if (s instanceof Error) { | ||
| // ... <scopeStart> <scope> <scopeEnd> ... | ||
| let ss = scopeStart(scanner) |
There was a problem hiding this comment.
I would be tempted to pull scope-start an scope-end into the scope parser, replacing the current logic that just ignores ( and ).
There was a problem hiding this comment.
I thought the same thing, and did so at first, the issue was I didn't see an example of returning multiple tokens. I can have it return an array or error, but that would be a new api in here. Is that the direction we want to go?
| @@ -187,6 +209,20 @@ function scope (scanner) { | |||
| } | |||
| } | |||
|
|
|||
| /* | |||
| * <scopeEnd> ::= ")" | |||
There was a problem hiding this comment.
Just to match the rest of the grammar, I'd probably make it <scope-start>/<scope-end>.
|
ok |
|
Don't know how this found it's way back into my inbox today 😆 Is this still something you'd like to see over finish line @wesleytodd? |
Figured I would take a stab at adding some tokens for the scope parens. This fails tests, and also adds new tokens not in the grammar yet, but I figured I would see if this is how you all were thinking of solving this before I added the other parts. Let me know what you think.