Skip to content
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

Allow variables starting with dollar sign ($) #313

Closed
nussjustin-hmmh opened this issue Sep 12, 2024 · 3 comments
Closed

Allow variables starting with dollar sign ($) #313

nussjustin-hmmh opened this issue Sep 12, 2024 · 3 comments

Comments

@nussjustin-hmmh
Copy link

Context

We'd like to use Twig-CS-Fixer for our Shopware projects (custom plugins / Symfony bundles).

Shopware uses Twig both directly from PHP, but also from JavaScript via twigjs/twig.js.

Since twig.js implements the same syntax as the reference Twig implementation, Twig-CS-Fixer mostly works fine, but we ran into a little problem:

Shopware uses a Variable / Function called $tc for translations (e.g. $tc('hello')). Note the dollar sign at the start.

While this is supported by twig.js, the reference Twig implementation and Twig-CS-Fixer do not support variables starting with a dollar sign.

For compatibility it would be nice for Twig-CS-Fixer to also allow the dollar sign at the start of variables.

Expected behavior

Variables starting with a dollar sign can be parsed.

Write here.

Actual behavior

Tokenizing fails.

 KO .../example.html.twig
 ------- --------------------------------------------------------------------- 
  FATAL   >>   | Unable to tokenize file: Unexpected character "$" at line 3.  
 ------- --------------------------------------------------------------------- 
@VincentLanglet
Copy link
Owner

Hi @nussjustin-hmmh, thanks for the contribution.

I'm not sure about the #314 change since I copy pasted the regex from Twig/Twig
https://github.com/twigphp/Twig/blob/6fa404ffdf1bbce56c51c708b2cd036205011e24/src/Lexer.php#L46

If you look at https://fiddle.nette.org/twig/#df4ba04b4a
it's reported as an error, so I'm not sure why I should support dollar sign for everyone and not add a way to customize some part of the Lexer.

IMHO we may split twig.js support and twig support.

If twig.js is based on twig, do you know where come from the extra support for $ in there code base (extra logic/different regex ?)

I might consider introducing (here

$tokenizer = new Tokenizer($twig);
) something like

$tokenizer = new Tokenizer($twig, $config->getTokenizerConfig());

but first I'd like to understand the way twigjs works.

@nussjustin-hmmh
Copy link
Author

Ok, I just realised that I didn't think this through.

Shopware does use twig.js but only for Blocks and not for the actual output. That is done via Vue.js, which means any {{ ... }} is actually handled by Vue and Shopware even overrides the twig.js config to ignore most tokens.

See https://github.com/shopware/shopware/blob/a28ca230ab22ec83c04feb4c4a6c23e61f591410/src/Administration/Resources/app/administration/src/core/factory/template.factory.js#L45-L50

That also means that running Twig-CS-Fixer on the templates is probably not even worth it trying.

Sorry for the trouble.

I'd say we close this issue and the PR. WDYT?

@VincentLanglet
Copy link
Owner

Indeed

@VincentLanglet VincentLanglet closed this as not planned Won't fix, can't repro, duplicate, stale Sep 12, 2024
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 a pull request may close this issue.

2 participants