Skip to content

Conversation

@yeonjuan
Copy link
Owner

Checklist

Description

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for template engine comment syntax by introducing an isComment flag to syntax configuration items. This allows template engines like Twig to properly handle comment blocks (e.g., {# ... #}) where inner template syntax should be ignored.

Key Changes:

  • Extended syntax configuration to support an isComment boolean flag in addition to the existing object-based format
  • Modified the parser to ignore template syntax inside comment blocks
  • Updated TWIG preset to use the new array-based syntax configuration with comment flag

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/template-syntax-parser/lib/types.ts Added SyntaxConfigItem and SyntaxConfig types to support both object and array-based syntax configurations with optional isComment flag
packages/template-syntax-parser/lib/template-syntax-parser.js Added normalization function to convert both syntax formats and refactored syntax pair sorting into a separate function
packages/template-syntax-parser/lib/parser.js Updated parser logic to track and handle comment syntax, ignoring nested template syntax within comment blocks
packages/template-syntax-parser/tests/template-syntax-parser.test.js Added TWIG configuration and test case for comment syntax handling
packages/template-syntax-parser/lib/exports.ts Exported new SyntaxConfig and SyntaxConfigItem types
packages/parser/lib/types.ts Updated ParserOptions to use the new SyntaxConfig type
packages/parser/lib/template-engine-syntax-preset.js Converted TWIG preset from object to array format with comment flag for {# #} syntax

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +227 to +228
this.syntaxStack.pop();
if (this.getPossibleCloseValueOf(top) === syntax.value) {
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stack is being popped before checking if the open and close values match. If they don't match (line 228), the top element has already been removed from the stack on line 227, which could lead to incorrect state. The pop should happen only after confirming the values match, or the logic should handle the mismatch case explicitly.

Suggested change
this.syntaxStack.pop();
if (this.getPossibleCloseValueOf(top) === syntax.value) {
if (this.getPossibleCloseValueOf(top) === syntax.value) {
this.syntaxStack.pop();

Copilot uses AI. Check for mistakes.
yeonjuan and others added 5 commits December 12, 2025 23:34
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@yeonjuan yeonjuan marked this pull request as ready for review December 12, 2025 15:45
@yeonjuan yeonjuan requested a review from Copilot December 12, 2025 15:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yeonjuan yeonjuan merged commit 8600ffc into main Dec 13, 2025
11 checks passed
@codecov
Copy link

codecov bot commented Dec 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.72%. Comparing base (68100e4) to head (b130d93).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #459   +/-   ##
=======================================
  Coverage   98.71%   98.72%           
=======================================
  Files          85       85           
  Lines        2810     2829   +19     
  Branches      772      773    +1     
=======================================
+ Hits         2774     2793   +19     
  Misses         36       36           
Flag Coverage Δ
unittest 98.72% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/template-syntax-parser/lib/parser.js 98.94% <100.00%> (+0.13%) ⬆️
...mplate-syntax-parser/lib/template-syntax-parser.js 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

[BUG] Parser fails when Twig syntax tokens appear inside Twig comments

2 participants