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

Add Drupal 'trans' tag to Twig linter. #2831

Merged
merged 4 commits into from
Jun 25, 2018

Conversation

wu-edward
Copy link

Fixes #2828.

Quick fix so that BLT Twig validation doesn't fail on the Drupal trans tag. Could possibly be more elegantly done.

This probably also needs tests.

Making this extensible to include custom/contrib module parsers/custom tags is to do as well.

Changes proposed:

  • Include Drupal TwigTransTokenParser into BLT Twig environment used for linting.

@wu-edward wu-edward force-pushed the add-twig-trans-tag branch from dbbfb1f to c203440 Compare May 29, 2018 15:25
@mikemadison13
Copy link
Contributor

@wu-edward can you give me some steps on how to validate and test this please?

@wu-edward
Copy link
Author

wu-edward commented May 30, 2018

@mikemadison13

  1. Add or edit a twig template in a custom Drupal theme in your BLT project.
  2. In the template, use the trans tag:
{% trans %}
  Text
{% endtrans %}

Also the plural version:

{% trans %}
  Hello {{ count }} world!
{% plural count %}
  Hello {{ count }} worlds!
{% endtrans %}
  1. Run blt tests:twig:lint:all
  2. With this commit, there should be no errors (and there should be errors without)
  3. You can also test that validation will fail if the trans tag is missing the end tag, or the plural tag is missing the parameter.

@ba66e77
Copy link
Contributor

ba66e77 commented Jun 25, 2018

I've confirmed the changes on my local. Updating branch with latest progress on 9.x then will merge.

@ba66e77 ba66e77 merged commit 4262d44 into acquia:9.x Jun 25, 2018
@wu-edward wu-edward deleted the add-twig-trans-tag branch September 10, 2018 15:29
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.

4 participants