trivago / prettier-plugin-twig-melody

Code formatting plugin for Prettier which can handle Twig/Melody templates
Apache License 2.0
155 stars 35 forks source link

Bug: Interpolation #21

Closed chasegiunta closed 4 years ago

chasegiunta commented 4 years ago

{% include "#{bar}" %} with singleQuotes set to true will output: {% include 'bar' %}

☝️ Couple of things here: Even if singleQuotes is set to true, ideally since double quotes are required for "string#{interpolation}" to work, the singleQuote setting should only convert double quotes to single quotes if #{ } is not present. https://twig.symfony.com/doc/3.x/templates.html#string-interpolation

The other other obvious weird thing going on is twig-melody is removing #{ ... } altogether.

twbartel commented 4 years ago

Very nice find! Fix is in v0.2.3, please have a look. The extraneous quotes were basically the same bug as in issue #20. The plugin now always uses double quotes around strings containing an interpolation, thanks for making me aware.

The fact that "#{bar}" turns into just the identifier bar is due to how the parser works. I consider this a valid transformation which is simpler than the original. Do you have strong objections to it? I'm just asking because fixing it would either require me to make changes in a separate project (Melody) or involve some rather ugly hack, and I feel the benefit would not outweigh the amount of effort.

chasegiunta commented 4 years ago

@twbartel thanks for your quick fixes! I probably won’t get around to testing until tomorrow.

I was initially confused by question. You’re saying in the context of “#{aSingleVariable}”, it will always transform into aSingleVariable because there’s no strings surrounding it on either side, thus keeping it within #{ ... } would be useless, correct?

If that’s the case, yep, totally agree with you. No need to change that behavior, I’d think.

twbartel commented 4 years ago

Yes, you understood correctly. When the string containing an interpolation consists of just one interpolation expression, it will be replaced by just the interpolated variable.

chasegiunta commented 4 years ago

Sounds good. This one also good! Thanks!