Skip to content

Conversation

@josdejong
Copy link
Owner

Fixes #3579

Solves math.evaluate('true?.3:.7') from being parsed as optional chaining and throwing an error.

@gwhitney
Copy link
Collaborator

Hmm, it's very special-case. I worry just slightly about the tokenizer becoming cluttered with lots of such special cases over time. This could be merged, but is it worth taking a second to consider whether any of the following would be cleaner:

  • Do this "checking for a number before tokenizing . as part of a delimiter" for all delimiters? Note that could regularize the parsing of the single-character . delimiter which is already so special that ., somewhat confusingly, isn't even included on the list of delimiters?
  • Allow delimiters to be regexps, so they can include negative lookahead assertions? That would eliminate the need to separately loop over 3, 2, and 1-character delimiters, and should also allow unifying the . single character delimiter?
  • Handle delimiters as a tree of characters rather than a list of 1, 2, and 3-character strings, so that as long as you are within a valid delimiter, you keep accumulating characters, except you never accumulate a . that is followed by a digit. That should also allow the handling of single-character . to be unified with all of the other delimiters.

Just some thoughts. If you prefer, I am willing to take a quick stab at any of the above, whichever sounds best to you -- none of them should take long, and I think any of them would overall simplify the tokenization rather that just adding another wrinkle to it. Let me know. Or I can just merge this as is if you prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Conditional expression parsing regression from #3547

3 participants