Remove the prepend_locale parameter from the parameters.yml file

[leofeyer]

Since we are no longer using the standard edition, we should remove the prepend_locale parameter from the parameters.yml file (see https://github.com/contao/core-bundle/pull/1519#discussion_r199476535). This implies that if you want to enable the feature, you have to create the app/config/config.yml file with the following content:

contao:
    prepend_locale: true

[aschempp]

I don't agree with this change. The parameter is a simplification for all users. If your example would be true, we could also remove any other parameter and set them in the bundle configs directly.

[leofeyer]

Have your read contao/core-bundle#1519? How is the parameter a simplification?

[m-vo]

I think the question is wether this should be an installation-dependent parameter or a config? And also I personally second the reasoning in your referenced comment. I remember there has been a discussion about this earlier. IIRC the reason for putting the thing in parametery.yml was to be able to deploy a single contao version to multiple clients with different configurations.

[fritzmg]

@aschempp imho it's not a simplification since you already have to configure your Contao application via the config.yml anyway, for things like the url suffix, jpeg quality etc. And this will eventually be done via the Contao Manager anyway (unless I am mistaken?).

[aschempp]

It's not about simplification, it's what people are already used to. There is no benefit in removing the parameter. If you like to use the config, simply do, and whatever you set in config.yml will override the parameter. But for anyone that already knows there's a config in parameter.yml, removing it would break everything (including installations on update).

[leofeyer]

As discussed in Mumble on July 5th, we can move the parameter from the skeleton/app/parameters.yml into the skeleton/app/config.yml file.

parameters:
    locale: en
    prepend_locale: false

We want to check if there is another backwards compatible solution that does not require to add the parameter to the config.yml file though.

[leofeyer]

I have found a way to preserve backwards compatibility in 405b1c61c217cc87ffdf78d52f1eb6b3249b6938.

[aschempp]

Also, setting a parameter does not correctly update the bundle configuration...

[leofeyer]

Also, setting a parameter does not correctly update the bundle configuration...

What exactly do you mean?

[aschempp]

You are setting a new parameter contao.prepend_locale. That's not the same as updating the bundle configuration. I think the correct approach would be to have the manager plugin implement ExtensionPluginInterface and append the respective configuration to the core bundle. Let me know if you want me to update your PR.

[leofeyer]

Sure, go ahead.

The ContaoKernel is certainly the wrong place for this change. It could be done in the Bundle or Plugin class.

I have put it there because we are also fixing the mailer_transport there. But I agree that both should be done in the plugin.

[leofeyer]

Also, I have put it there because we need to check directly after the parameters.yml file has been loaded. Once the configuration has been merged, it is too late.

[leofeyer]

Thank you for your help on this one @aschempp. Much appreciated.

[leofeyer]

Unfortunately, this does not work as expected:

In ArrayNode.php line 311:

  [Symfony\Component\Config\Definition\Exception\InvalidConfigurationException]  
  Unrecognized option "contao" under "contao"                                    

Exception trace:
 Symfony\Component\Config\Definition\ArrayNode->normalizeValue() at /vendor/symfony/symfony/src/Symfony/Component/Config/Definition/BaseNode.php:282
 Symfony\Component\Config\Definition\BaseNode->normalize() at /vendor/symfony/symfony/src/Symfony/Component/Config/Definition/Processor.php:33
 Symfony\Component\Config\Definition\Processor->process() at /vendor/symfony/symfony/src/Symfony/Component/Config/Definition/Processor.php:50
 Symfony\Component\Config\Definition\Processor->processConfiguration() at /vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/Extension/Extension.php:96
 Symfony\Component\DependencyInjection\Extension\Extension->processConfiguration() at /vendor/symfony/symfony/src/Symfony/Component/HttpKernel/DependencyInjection/ConfigurableExtension.php:35
 Symfony\Component\HttpKernel\DependencyInjection\ConfigurableExtension->load() at /vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/Compiler/MergeExtensionConfigurationPass.php:71
 Symfony\Component\DependencyInjection\Compiler\MergeExtensionConfigurationPass->process() at /vendor/symfony/symfony/src/Symfony/Component/HttpKernel/DependencyInjection/MergeExtensionConfigurationPass.php:39
 Symfony\Component\HttpKernel\DependencyInjection\MergeExtensionConfigurationPass->process() at /vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/Compiler/Compiler.php:141
 Symfony\Component\DependencyInjection\Compiler\Compiler->compile() at /vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/ContainerBuilder.php:778
 Symfony\Component\DependencyInjection\ContainerBuilder->compile() at /vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php:641
 Symfony\Component\HttpKernel\Kernel->initializeContainer() at /vendor/contao/manager-bundle/src/HttpKernel/ContaoKernel.php:220
 Contao\ManagerBundle\HttpKernel\ContaoKernel->initializeContainer() at /vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php:135
 Symfony\Component\HttpKernel\Kernel->boot() at /vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php:152
 Symfony\Component\HttpKernel\Kernel->reboot() at /vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Command/CacheClearCommand.php:200
 Symfony\Bundle\FrameworkBundle\Command\CacheClearCommand->warmup() at /vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Command/CacheClearCommand.php:134
 Symfony\Bundle\FrameworkBundle\Command\CacheClearCommand->execute() at /vendor/symfony/symfony/src/Symfony/Component/Console/Command/Command.php:251
 Symfony\Component\Console\Command\Command->run() at /vendor/symfony/symfony/src/Symfony/Component/Console/Application.php:964
 Symfony\Component\Console\Application->doRunCommand() at /vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Console/Application.php:86
 Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() at /vendor/symfony/symfony/src/Symfony/Component/Console/Application.php:248
 Symfony\Component\Console\Application->doRun() at /vendor/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Console/Application.php:74
 Symfony\Bundle\FrameworkBundle\Console\Application->doRun() at /vendor/symfony/symfony/src/Symfony/Component/Console/Application.php:148
 Symfony\Component\Console\Application->run() at /vendor/contao/manager-bundle/bin/contao-console:44

The error is caused here:

https://github.com/contao/manager-bundle/blob/171ef0e28d13128231b10ee61fa64f1cf2dad5a3/src/ContaoManager/Plugin.php#L257-L261

And I'm pretty sure the error did not occur 14 days ago. 🤔

[leofeyer]

The problem is that the manager bundle is loaded before the core bundle, so there is no bundle handling the contao configuration namespace yet. @aschempp Can we move this to a compiler pass?

[aschempp]

I don't see how that should be a problem?

[leofeyer]

Did you read the error message?

[aschempp]

Well it says

Unrecognized option "contao" under "contao"

So maybe we should not add the parameter to an array key contao but to the root of the returned configuration? Not sure how to unit-test that :(

[leofeyer]

So maybe we should not add the parameter to an array key contao but to the root of the returned configuration?

That was it! Fixed in 09ad3f4c01a957dbfdb937bb9c63517bdcf6298e now. Thank you @aschempp.