Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Smarty 4 Suppress Deprecation Notices for Unregistered Callables in Templates using muteUndefinedOrNullWarnings function #1067

Conversation

dipeshsukhia
Copy link

@dipeshsukhia dipeshsukhia commented Sep 18, 2024

Hi @wisskid

As per discussion #967, Smarty 4.5 has introduced deprecation notices to warn users about the usage of unregistered callables in templates.

To prevent these warnings from cluttering the logs, we've implemented a fix that suppresses these deprecation notices by utilizing the muteUndefinedOrNullWarnings function for Smarty 4.5.

This change ensures that deprecated notices for unregistered callables are properly handled, preventing unnecessary interruptions
in Smarty 4.5.

@dipeshsukhia dipeshsukhia changed the base branch from master to support/4 September 18, 2024 04:31
@dipeshsukhia dipeshsukhia force-pushed the smarty-4-suppresses-Smarty-register-class-plugin-error-in-muteUndefinedOrNullWarnings branch from 1b7dcac to 925a334 Compare September 18, 2024 05:47
@wisskid
Copy link
Member

wisskid commented Sep 19, 2024

Why would Smarty throw a notice first and then catch and supress it again? The notices are there for a reason.

If you don't want the depreciation notices, just change your error logging (or reporting) levels.

@wisskid wisskid closed this Sep 19, 2024
@dipeshsukhia
Copy link
Author

Hi @wisskid

Thank you for your feedback!

The goal of suppressing deprecation notices at the package level using muteUndefinedOrNullWarnings is to give developers flexibility without having to modify global error logging settings. This approach is especially helpful for those working with legacy code or third-party integrations, allowing them to maintain clean logs without missing critical issues.

This solution specifically targets unregistered callables while preserving the ability to manage other important notices.

@wisskid
Copy link
Member

wisskid commented Sep 19, 2024

I think I see what you mean, but am not convinced that your approach is correct.

First of all: muteUndefinedOrNullWarnings has a very different goal. It mutes warnings (not notices) and does so in order to maintain identical behavior when switching from PHP7 to PHP8. Nobody should have any reason to expect that calling this method would drop the newly added deprecation notices in Smarty. If there was an important use case that calls for preventing these notices from ending up in logs, there should be a (separate) setting that would prevent the notices from being thrown in the first place.

However, the goal "to maintain clean logs without missing critical issues" seems quite arbitrary. You seem to feel that the notices are not critical and other notices are more important, but other people might disagree or have additional preferences.

it seems to me that you could just as easily add a custom error handler to the application that is using Smarty, temporarily change your error logging settings when calling $smarty->display('template.tpl') or configure your logging output stream to drop certain messages if this is something you need.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants