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

Enable custom Twig tags #1

Closed
urbantrout opened this issue Oct 26, 2019 · 17 comments
Closed

Enable custom Twig tags #1

urbantrout opened this issue Oct 26, 2019 · 17 comments

Comments

@urbantrout
Copy link
Contributor

I get an error when I use CMS specific custom Twig tags, such as the nav and endnav tag of Craft CMS (https://docs.craftcms.com/v2/templating/nav.html)

[error] Expected a known tag such as
[error] - autoescape
[error] - block
[error] - do
[error] - embed
[error] - extends
[error] - filter
[error] - flush
[error] - for
[error] - from
[error] - if
[error] - import
[error] - include
[error] - macro
[error] - set
[error] - spaceless
[error] - use
[error] - moun

Would be nice to extend this rule

@twbartel
Copy link
Collaborator

Hi @urbantrout, I'm counting 14 custom Twig tags in the Craft documentation. I'm not sure if I will find the time to implement them all. Can you prioritize them, maybe? If I do the first few, maybe you can try one of them yourself?

@chasegiunta
Copy link

@twbartel I may be able to help with this if you do implement a few...

I think the ultimate issue may be (not only in Craft but in others) that plugins, 3rd party add-ons, etc. can add their own custom twig tags, so it may be impossible to whitelist them all within this plugin.

Not sure if a prettier twig whitelist option is possible or not...?

@twbartel
Copy link
Collaborator

twbartel commented Jan 21, 2020

Hey @chasegiunta, that's very nice, thanks.

The thing is, whitelisting alone is not enough. Every Twig tag potentially comes with its own structure. For example, while a {% set ... %} tag is just this one tag, an {% if ... %} tag can have an {% else %}, any number of {% elseif ... %} parts, etc. You really need a custom parser for each one.

In principle, this is taken care of, because this Prettier plugin is itself pluggable. For example, in our codebase, we have an {% icon ... %} tag and a plugin for it. We use it like so:

const compatPlugin = require('../melody-plugin-orchestra-compat');
const prettier = require('prettier');
const { group, concat, line, indent } = prettier.doc.builders;
const {
    STRING_NEEDS_QUOTES,
    EXPRESSION_NEEDED,
} = require('prettier-plugin-twig-melody');

const printIconTag = (node, path, print) => {
    ...
};

module.exports = {
    melodyExtensions: [compatPlugin],
    printers: {
        IconTag: printIconTag,
    },
};

So, a plugin to this plugin exports melodyExtensions (which contain the parser(s)) and printers, which take care of the printing. In this case, both of them are defined in the project which uses the Prettier plugin, not in the Prettier plugin itself. I should probably add some documentation on how to write these plugins to the plugin...

@twbartel
Copy link
Collaborator

Hello @chasegiunta and @urbantrout, I started a separate repository which is an extension to this plugin: https://github.com/twbartel/prettier-twig-craft-cms

This can now be pulled into a project as an additional dev dependency, in case this project uses Craft CMS.

So far, I implemented only the nav tag. The upside is, the project structure is still very small. It should give you a first idea how to add your own Twig tags. It's not ideal, and still a lot of boilerplate, but maybe you already want to have a look.

@chasegiunta
Copy link

@twbartel Holy cow, after looking at that first nav tag, I fear I may have been over confident in my abilities to reimplement other tags. Just that one nav is 200 lines of an unfamiliar API, whew!

Let me ask you this, and forgive me if I'm being naive but, rather than explicitly building a plugin/extension for each tag, could they be vague enough to where those that follow a similar structure could be all in one?

There's several that follow the same tag structure as {% extends " ... " %}.

The css and js tags both follow the same structure as twig's native {% autoescape %} tags, etc.

@twbartel
Copy link
Collaborator

You are right, this can definitely be abstracted and simplified, maybe even to the point where you can add a whitelist in the configuration - at least, for simple tags like {% redirect %} or {% requireLogin %}. I agree that the hurdle to add new tags is currently too high.

@sjelfull
Copy link

sjelfull commented Feb 8, 2020

Cheers for at least trying to take this on @twbartel ! Here is hoping for some kind of universal whitelist.

Just tested this, and sadly it broke on most of 100 templates in Craft.

@twbartel
Copy link
Collaborator

Update to everyone interested: I'm very close to being able to handle custom Twig tags in an easy way. Waiting for trivago/melody#154 to be merged into Melody master, then I can merge some functionality into this project. Stay tuned!

@chasegiunta
Copy link

@twbartel That's great news! Super appreciative of your work!

@twbartel
Copy link
Collaborator

PR prepared, but not ready to merge yet: #35

How does this sound?

@sjelfull
Copy link

Sounds very nice and pragmatic 🥳

@twbartel
Copy link
Collaborator

twbartel commented Mar 1, 2020

Version 0.4.0 is out and should fix this issue. @sjelfull @chasegiunta thanks for your patience!

@twbartel
Copy link
Collaborator

twbartel commented Mar 2, 2020

@sjelfull can you give me some feedback if it's working with Craft for you now?

@sjelfull
Copy link

sjelfull commented Mar 5, 2020

Thanks, seems to be a huge improvement! I still have issues in most files. Will sort through what is syntax issues (like unclosed tags etc.) and then circle back.

@rbrv
Copy link

rbrv commented Mar 9, 2020

Great work!

Got this error while testing:

Error

[error] templates/shop/products/_item.twig: Error: ERROR: Invalid token
[error]   2 |                         <select name="purchasableId" id="purchasableId" class="purchasableId">
[error]   3 |                             {%- for purchasable in product.getVariants() -%}
[error] > 4 |                                 <option {% if not purchasable.isAvailable %}disabled{% endif %}>
[error]     |                                         ^
[error]   5 |                                     {{ purchasable.description }}
[error]   6 |                                     {{
[error]   7 |                                         purchasable.salePrice|commerceCurrency(
[error] 
[error] A tag must consist of attributes or expressions. Twig Tags are not allowed.
[error]     at TokenStream.error (/mytestproject/node_modules/melody-parser/lib/index.js:1359:22)
[error]     at Parser.error (/mytestproject/node_modules/melody-parser/lib/index.js:830:21)
[error]     at Parser.matchAttributes (/mytestproject/node_modules/melody-parser/lib/index.js:818:22)
[error]     at Parser.matchElement (/mytestproject/node_modules/melody-parser/lib/index.js:726:14)
[error]     at Parser.parse (/mytestproject/node_modules/melody-parser/lib/index.js:627:32)
[error]     at Object.parse (/mytestproject/node_modules/melody-extension-core/lib/index.js:1187:36)
[error]     at Parser.matchTag (/mytestproject/node_modules/melody-parser/lib/index.js:857:29)
[error]     at Parser.parse (/mytestproject/node_modules/melody-parser/lib/index.js:610:32)
[error]     at Parser.matchElement (/mytestproject/node_modules/melody-parser/lib/index.js:736:41)
[error]     at Parser.parse (/mytestproject/node_modules/melody-parser/lib/index.js:627:32)

Source file


                        <select name="purchasableId" id="purchasableId" class="purchasableId">
                            {%- for purchasable in product.getVariants() -%}
                                <option {% if not purchasable.isAvailable %}disabled{% endif %}>
                                    {{ purchasable.description }}
                                    {{
                                        purchasable.salePrice|commerceCurrency(
                                            cart.currency
                                        )
                                    }}
                                </option>
                            {%- endfor -%}
                        </select>

.prettierrc

{
    "tabWidth": 4,
    "plugins": ["./node_modules/prettier-plugin-twig-melody"],
    "twigMultiTags": [
        "nav,endnav",
        "switch,case,default,endswitch",
        "ifchildren,endifchildren",
        "cache,endcache"
    ]
}

@twbartel
Copy link
Collaborator

@rbrv Gonna look into that. However, it seems to be a different issue, so could you open a new one, please?

@rbrv
Copy link

rbrv commented Mar 10, 2020

Sure: #37

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

No branches or pull requests

5 participants