-
Notifications
You must be signed in to change notification settings - Fork 53
Fix nested curly braces in template strings #1539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com>
src/lexer/Lexer.ts
Outdated
| let braceDepth = 0; | ||
| while (!this.isAtEnd()) { | ||
| if (this.check('}')) { | ||
| if (braceDepth === 0) { | ||
| // This is the closing brace for the template expression | ||
| break; | ||
| } else { | ||
| // This is a regular right brace inside the expression | ||
| braceDepth--; | ||
| this.start = this.current; | ||
| this.advance(); | ||
| this.addToken(TokenKind.RightCurlyBrace); | ||
| } | ||
| } else if (this.check('{')) { | ||
| // Track nested braces | ||
| braceDepth++; | ||
| this.start = this.current; | ||
| this.scanToken(); | ||
| } else { | ||
| this.start = this.current; | ||
| this.scanToken(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that we've essentially inlined the main loop inside template strings. this will be difficult to maintain in the future. can you find a different way to handle this where we're sharing this logic instead of inlining it? Or a different approach entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right about the maintainability concerns. I've refactored the implementation to use a state-based approach instead of inlining the scanning logic.
Changes made:
- Moved curly brace handling from
tokenKindMaptotokenFunctionMapwith logic to check template expression state - Added
isInTemplateExpressionandtemplateExpressionBraceDepthinstance variables - Modified
templateStringmethod to use existingscanTokenlogic instead of custom loop - Added state save/restore for nested template strings
This approach reuses all existing scanning logic, eliminating code duplication and making it much more maintainable. All tests continue to pass.
Commit: 3829c16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's pretty creative. However, what about nested template expressions? This only supports a max depth of one. But we could have stuff like this:
print `one${`two${`three${`four`}`}`}`Please update the logic to handle this more complicated situation while still keeping it maintainable and reusable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've implemented a stack-based approach to handle nested template expressions. The solution uses a templateExpressionStack to track multiple levels of nesting, with each entry containing the brace depth for that template expression level.
The implementation:
- Moves curly brace handling from
tokenKindMaptotokenFunctionMapwith logic to check template expression state - Uses a stack where each level tracks its own brace depth independently
- Properly handles arbitrary nesting depth like
\one${two${three${four}`}`}``
The approach is partially working but I'm still debugging an issue where template string quasi processing is capturing } characters that should be processed as TemplateStringExpressionEnd tokens. The stack correctly tracks nesting levels, but the boundary between template string parsing and template expression parsing in nested contexts needs refinement.
Commit: 87c7e14
Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com>
Co-authored-by: TwitchBronBron <2544493+TwitchBronBron@users.noreply.github.com>
Fixes a bug in the lexer where nested curly braces in template strings were not being parsed correctly. The lexer would treat the first
}it encountered as the end of a template expression, instead of properly tracking brace depth to find the actual closing brace.Before this fix:
After this fix:
Implementation:
The fix implements proper brace depth tracking within template string expressions:
{, increment depth and process asLeftCurlyBracetoken}, if depth > 0, decrement depth and process asRightCurlyBracetoken}treated asTemplateStringExpressionEndTest Coverage:
Added comprehensive test cases including:
`${{}}``${{a: {b: 1}}}``${arr[0]} and ${{key: value}}`All existing tests continue to pass (2441 passing, 0 failing).
Fixes #1538.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.