Code: Select all
public function is_enableable()
{
if (this extension should not be enabled)
{
$this->container->get('user')->add_lang_ext('vendor/extension', 'errormessage');
throw new extension_exception($this->container->get('user')->lang('MEANINGFUL_ERROR_MESSAGE']);
}
return true;
}
Code: Select all
try
{
$extension_manager->enable(); // enable or wathever
}
catch(\phpbb\extensions\exception $e)
{
$lang_key = $e->getMessage();
trigger_error($user->lang($lang_key).adm_back_link(...));
}
catch(\phpbb\extensions\extension_exception $e)
{
trigger_error($e->getMessage().adm_back_link(...));
}
Code: Select all
try
{
$extension_manager->enable(); // enable or wathever
}
catch(\phpbb\extensions\exception $e)
{
$lang_key = $e->getMessage();
$io->error($user->lang($lang_key));
}
catch(\phpbb\extensions\extension_exception $e)
{
$io->error($e->getMessage());
}
Code: Select all
public function is_enableable()
{
if (this extension should not be enabled)
{
$this->container->get('user')->add_lang_ext('vendor/extension', 'errormessage');
throw new extension_exception('MEANINGFUL_ERROR_MESSAGE', array(any relevant args));
}
return true;
}
Code: Select all
try
{
if (!$this->ext_manager->get_extension($ext_name)->is_enableable())
{
throw new extension_exception('EXTENSION_NOT_ENABLEABLE');
}
}
catch (exception_interface $e)
{
$message = call_user_func_array(array($this->user, 'lang'), array_merge(array($e->getMessage()), $e->get_parameters()));
trigger_error($message, E_USER_WARNING);
}
Code: Select all
try
{
if (!$extension->is_enableable())
{
return false;
}
}
catch (exception_interface $e)
{
return false;
}
Code: Select all
$this->container->get('user')->add_lang_ext('vendor/extension', 'errormessage');
Correction: no extension service is being loaded, only "core" service(s) (user or language) are loaded from the container. And the language file does not depend on the enableability (if that is a word) of the extension, but only on the availability of the extension (already tested and validated) and the language file itself (responsibility of the developer and the forum admin). So this code is perfectly fine, and there are a number of extensions showing it and using it without any issue.VSE wrote: ↑Fri Apr 21, 2017 6:33 am When an extension is not installed, it's not part of the container, so you can't load any of its services...you can not:
Also, in 3.3, extensions will be installed through composer within the ACP so composer error messages will be output instead of what's being discussed here.Code: Select all
$this->container->get('user')->add_lang_ext('vendor/extension', 'errormessage');
using ext.php to do any of this has always just been a temporary workaround until composer is supported. https://github.com/phpbb/phpbb/pull/3909
Yes, except trigger error gives an error/warning in the CLI.RMcGirr83 wrote: ↑Fri Apr 21, 2017 2:45 pm This seems to work just fine in 3.1 and 3.2?
https://github.com/rmcgirr83/phpBB-3.1- ... er/ext.php
But how many actually install, or need to install, extensions using CLI?
Speaking for myself, I haven't used it ever.
After a rather long investigation and very useful comments from VSE, this was the final conclusion:javiexin wrote: ↑Sat Mar 11, 2017 11:44 pm Hello,
I would like to know what is the right way to display "meaningful" messages for the user in ext.php.
What I was doing is:
Well, this works just fine inCode: Select all
public function is_enableable() { if (this extension should not be enabled) { $this->container->get('user')->add_lang_ext('vendor/extension', 'errormessage'); trigger_error($this->container->get('user')->lang('MEANINGFUL_ERROR_MESSAGE'], E_USER_WARNING); } return true; }
acp_extension
, failing when it should and showing the message correctly. But if I runphpbbcli extension:enable
then there is a kind of stack trace (with the error message showing twice), and what is worse, the extension is enabled. Maybe I have to put areturn false;
aftertrigger_error
but I think that an error should be enough to interrupt code...
Is this a CLI error/limitation, or is something that I am not doing quite right?
Special facts collected during the discussion:javiexin wrote: ↑Tue Mar 14, 2017 6:07 pm What I will finally use as a pattern, isTested both with the ACP and CLI, works fine.Code: Select all
public function is_enableable() { $is_enableable = <condition that must be met>; if (!$is_enableable) { $this->container->get('user')->add_lang_ext('vendor/extension', 'errormessage'); @trigger_error($this->container->get('user')->lang['MEANINGFUL_ERROR_MESSAGE'], E_USER_WARNING); } return $is_enableable; }
trigger_error
stops any further execution within the ACP, but it continues from where it left it in the CLI, so you MUST return false
after calling trigger_error
for the function to work in the CLI as well, not assuming that trigger_error
would stop execution.'@'
operator.functions.php
; sample working code was provided (viewtopic.php?p=14685191#p14685191).I would think the worthless captchas would be the first to go. They haven't worked correctly and/or have been beaten for years.
I agree with both of your comments.Paul wrote: ↑Fri Apr 21, 2017 6:02 pm The problem is that a WARNING never stops execution. thats why it is a warning . phpBB is misusing the trigger_errors to display messages to the user, so it is the GUI that actually needs fixing, not the CLI (That is how the behaviour should be).
The is_enableable was never really meant to have trigger_error called, but to only return a boolean. That boolean determines if a extension can be enabled or not.
trigger_error(...,E_USER_WARNING);
does stop the execution in the GUI. If we were to look at it from that "correct" viewpoint, then quite a lot of code in the ACP would be "incorrect" as it assumes that after trigger_error, execution is stopped. The behaviour of trigger_error should be consistent across phpBB: if it stops execution in the GUI, it should do so in the CLI, and if it does not stop in the CLI, then it should not stop in the GUI. The source of the problem is inconsistency. And by the way, this is also heavily used in extensions, so if this were to be changed, it would break a lot of them (most of the ones that have an ACP module for that matter).trigger_error
) seems to me quite vague so that someone may act accordingly.trigger_error
, then the exception option proposed by rubencm is quite correct. It is flexible, requires minimum code changes in the core, and is simple enough for extension authors to implement (throw an exception with the right error instead of return false in is_enableable
). What other alternatives exist?Because we are not rewriting phpBB within 1 release, so you have a some code that is already rewritten (Or just new), like the CLI, and some code that is old, that can behave differently.javiexin wrote: ↑Fri Apr 21, 2017 6:28 pmI agree with both of your comments.Paul wrote: ↑Fri Apr 21, 2017 6:02 pm The problem is that a WARNING never stops execution. thats why it is a warning . phpBB is misusing the trigger_errors to display messages to the user, so it is the GUI that actually needs fixing, not the CLI (That is how the behaviour should be).
The is_enableable was never really meant to have trigger_error called, but to only return a boolean. That boolean determines if a extension can be enabled or not.
However,trigger_error(...,E_USER_WARNING);
does stop the execution in the GUI. If we were to look at it from that "correct" viewpoint, then quite a lot of code in the ACP would be "incorrect" as it assumes that after trigger_error, execution is stopped. The behaviour of trigger_error should be consistent across phpBB: if it stops execution in the GUI, it should do so in the CLI, and if it does not stop in the CLI, then it should not stop in the GUI. The source of the problem is inconsistency. And by the way, this is also heavily used in extensions, so if this were to be changed, it would break a lot of them (most of the ones that have an ACP module for that matter).
In general within the description of the extension .Now, if we take the above conversation aside for a minute, we have yet another issue: in what "correct" way may an extension communicate an end user the reason why it is not accepting being enabled? The "EXTENSION_IS_NOT_ENABLEABLE" message that the ACP shows (by the way, usingtrigger_error
) seems to me quite vague so that someone may act accordingly.
Like said before, the real solution will be the composer integration, which is coming in 3.3.
If we rule outtrigger_error
, then the exception option proposed by rubencm is quite correct. It is flexible, requires minimum code changes in the core, and is simple enough for extension authors to implement (throw an exception with the right error instead of return false inis_enableable
). What other alternatives exist?
-javiexin