Skip to content

Add Twig pre-processor#20198

Merged
RobinMalfait merged 2 commits into
mainfrom
fix/issue-19458
Jun 4, 2026
Merged

Add Twig pre-processor#20198
RobinMalfait merged 2 commits into
mainfrom
fix/issue-19458

Conversation

@RobinMalfait

Copy link
Copy Markdown
Member

This PR fixes an issue where addClass(opacity-50) and removeClass(opacity-50) in Twig templates isn't extracted properly.

This fixes that by adding a pre-processor for .twig files. This is a simple initial implementation where we drop the ( and ) from the addClass and removeClass functions. We don't do any special handling around escaped characters or parenthesis inside of strings. We will add them when there is a use case for it. Until then, we'll keep it simple.

If the real API would've been addClass('opacity-50') then it would've worked out of the box.

A workaround you can use today is by using spaces addClass( opacity-50 ) that works as well.

Fixes: #19458
Closes: #20110

Test plan

  1. Added new tests for twig extraction
  2. Added tests with nested ( and ) e.g. addClass(p-(--value)) which should properly extarct p-(--value)

@RobinMalfait RobinMalfait requested a review from a team as a code owner June 4, 2026 09:33
@greptile-apps

greptile-apps Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge — the pre-processor only substitutes characters in place (output length is always equal to input length), and the bracket-depth guard logic is correct.

The underflow concern raised in earlier review rounds is fully addressed with explicit lower-bound guards before the slice operations. The bracket-depth stack correctly handles nested CSS-variable syntax and leaves all non-addClass/removeClass parentheses untouched. Tests exercise the original bug report, nested variable syntax, multi-class arguments, false-positive avoidance (addAttribute), and real-world component markup.

No files require special attention.

Reviews (2): Last reviewed commit: "update changelog" | Re-trigger Greptile

Comment thread crates/oxide/src/extractor/pre_processors/twig.rs
@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

This PR adds a Twig pre-processor module that rewrites addClass(...) and removeClass(...) calls by replacing opening and closing parentheses with spaces. The new Twig struct implements the PreProcessor trait with a process method that iterates through input bytes, detects these function patterns, maintains a parenthesis stack to track nesting, and normalizes delimiters. The pre-processor is declared and exported through the module system, integrated into the scanner to recognize .twig file extensions, and covered by unit tests and integration tests. A changelog entry documents the feature.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add Twig pre-processor' is clear and specific, directly summarizing the main change of adding a new pre-processor module for Twig files.
Description check ✅ Passed The description explains the issue being fixed (addClass/removeClass extraction in Twig), the solution (pre-processor), implementation details, and test plan - all related to the changeset.
Linked Issues check ✅ Passed The PR meets the objectives from issues #19458 and #20110 by implementing a Twig pre-processor that extracts classes from addClass/removeClass directives and includes tests.
Out of Scope Changes check ✅ Passed All changes are in-scope: CHANGELOG entry, new Twig pre-processor module, module registration, scanner integration, and tests - all directly address the stated objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@RobinMalfait RobinMalfait merged commit 0612ddc into main Jun 4, 2026
9 checks passed
@RobinMalfait RobinMalfait deleted the fix/issue-19458 branch June 4, 2026 09:44
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.

Source classes are not detected correctly when specified as data-loading="addClass(opacity-50)"

1 participant