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 missing template name and line no to some deprecation messages #3031

Merged
merged 1 commit into from May 28, 2019

Conversation

yceruto
Copy link
Contributor

@yceruto yceruto commented May 27, 2019

before:
twig-before

after:
twig-after

@stof
Copy link
Member

stof commented May 27, 2019

Wasn't it done on purpose in the past to avoid breaking the grouping @nicolas-grekas ?

@nicolas-grekas
Copy link
Contributor

Yes, that's true. The goal is to limit the number of repetitive messages.
But I guess there is not a huge list of different file+line, so this should be fine?

@stof
Copy link
Member

stof commented May 27, 2019

Well, every time this deprecation is triggered, the file+line will be different (at least if you write your templates in a readable way rather than putting them in a single very long line).

@yceruto
Copy link
Contributor Author

yceruto commented May 27, 2019

I'm fine with adding the template name only if we want to keep the group per template

@yceruto
Copy link
Contributor Author

yceruto commented May 27, 2019

Note that the real problem here is that neither "Show context" nor "Show trace" show the affected template for those deprecation notices, so it's very hard to find which template contains the error.

@fabpot
Copy link
Contributor

fabpot commented May 28, 2019

Not related, but I've just released a new version of SwiftmailerBundle to get rid of these deprecation notices.

@fabpot
Copy link
Contributor

fabpot commented May 28, 2019

I think we need to add this information as Twig templates are complied to PHP, so without doing it, it's quite complex to find the usage (at least if you don't know how to use grep :)). So 👍 on my side.

@yceruto yceruto force-pushed the improving_deprecation_msg branch from c54f6af to 8cd300f Compare May 28, 2019 12:53
@fabpot
Copy link
Contributor

fabpot commented May 28, 2019

Thank you @yceruto.

@fabpot fabpot merged commit 8cd300f into twigphp:2.x May 28, 2019
fabpot added a commit that referenced this pull request May 28, 2019
… messages (yceruto)

This PR was merged into the 2.x branch.

Discussion
----------

Add missing template name and line no to some deprecation messages

before:
![twig-before](https://user-images.githubusercontent.com/2028198/58422946-e1346080-8061-11e9-80e1-b75418fe03e9.png)

after:
![twig-after](https://user-images.githubusercontent.com/2028198/58422955-e85b6e80-8061-11e9-9c20-2bbf0ccb5009.png)

Commits
-------

8cd300f Add missing template name and line no to some deprecation messages
@yceruto yceruto deleted the improving_deprecation_msg branch May 28, 2019 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants