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.

@josdejong
Copy link
Owner Author

josdejong commented Nov 5, 2025

Agree, this is an ugly hotfix. We really need to come up with something better.

I would appreciate if you can try out improving the tokenization Glen! It can be that regexps will come in handy here (utilizing lastIndex). Or maybe we can replace the delimiter map with a list objects having a name and a test function, which can then hold more complex logic.

In the meantime, I'll merge this fix, so we can publish a new version with optional chaining and an important fixes for a bug introduced in v15.0.0. I've added a TODO in the code.

@josdejong josdejong merged commit b15904f into develop Nov 5, 2025
8 checks passed
@josdejong josdejong deleted the fix/conditional-or-optional-chaining branch November 5, 2025 11:22
@josdejong
Copy link
Owner Author

Published now in v15.1.0.

@gwhitney
Copy link
Collaborator

gwhitney commented Nov 6, 2025

OK, I will bump up the priority on reviving #3423 but on top of current develop branch, and make sure to include smoother delimiter tokenization. I think it will be a fresh start because a fair amount has changed since then, but should be faster having done it once. Hope to get a start on it tonight.

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