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

Fix the translation commands when a template contains a syntax error #34711

Merged
merged 1 commit into from Nov 30, 2019

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Nov 29, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #34586
License MIT
Doc PR n/a

When using debug:translation or translation:update, we should catch exceptions to avoid breaking the command. It was not really an issue before Symfony 4.4/5 as we didn't have templates in the core that use features from optional dependencies.

@stof
Copy link
Member

stof commented Nov 29, 2019

This is not the right fix IMO: this catches the errors at the level of a Twig top-level folder. So it will ignore any other template in the same folder if one of them has an issue (so ignoring twig-bridge entirely for instance). We should rather catch things for each template separately, to keep processing others.

@stof
Copy link
Member

stof commented Nov 29, 2019

Also, doing this in the extractor would also mean that it applies to other tools using the extractor.

@fabpot
Copy link
Member Author

fabpot commented Nov 29, 2019

I would not do it at the extractor level as it would mean that we would not be able to emit a notice (which might be fine after all). Ignoring a directory is not a problem IMO as there is a syntax error that should be fixed if this is a directory in the user project, and for built-in templates, we don't really care.

@stof
Copy link
Member

stof commented Nov 29, 2019

well, bundles might have some templates used for specific case like that. And this extractor would then skip the whole bundle templates.

The translation extractor is not responsible for linting templates (we have a command for that), and ignoring valid templates makes it less useful.

@fabpot
Copy link
Member Author

fabpot commented Nov 29, 2019

@stof code updated :)

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Nov 29, 2019
@nicolas-grekas
Copy link
Member

But tests should be updated :)

fabpot added a commit that referenced this pull request Nov 30, 2019
…ntax error (fabpot)

This PR was merged into the 3.4 branch.

Discussion
----------

Fix the translation commands when a template contains a syntax error

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #34586
| License       | MIT
| Doc PR        | n/a

When using `debug:translation` or `translation:update`, we should catch exceptions to avoid breaking the command. It was not really an issue before Symfony 4.4/5 as we didn't have templates in the core that use features from optional dependencies.

Commits
-------

7f803bc Fix the translation commands when a template contains a syntax error
@fabpot fabpot merged commit 7f803bc into symfony:3.4 Nov 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants