Page MenuHomePhabricator

ExtensionRegistry does not allow complete override of config variables e.g. flat arrays
Closed, DeclinedPublic

Description

It'd be really useful if there was a way to have a variable completely override an array. ie we set defaults in the extension config, but in LocalSettings (or CommonSettings et al) we can wipe that out, not use it, and use whatever we've defined there

Related Objects

Event Timeline

Reedy created this task.Aug 11 2016, 12:04 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 11 2016, 12:04 AM
Jdlrobson triaged this task as High priority.Apr 26 2017, 5:56 PM
Jdlrobson added a project: Readers-Web-Backlog.
Jdlrobson added a subscriber: Jdlrobson.

Not only useful, but essential. The current behaviour is highly misleading.

If extension.json defines:
wgMoo = ['a']

and in LocalSettings in production I put
wgMoo = [];

wgMoo should not be ['a'].

Jdlrobson renamed this task from Allow complete override of config variables to ExtensionRegistry does not allow complete override of config variables e.g. flat arrays.Apr 26 2017, 6:29 PM

Change 374042 had a related patch set uploaded (by EddieGP; owner: EddieGP):
[mediawiki/core@master] ExtensionRegistry: Add 'override' merge strategy

https://gerrit.wikimedia.org/r/374042

I've run into the same thing when working on T62419 and don't really want to wait for https://gerrit.wikimedia.org/r/#/c/335383/ to be merged to see this fixed, so I've pushed another patch for that.

I've run into the same thing when working on T62419 and don't really want to wait for https://gerrit.wikimedia.org/r/#/c/335383/ to be merged to see this fixed, so I've pushed another patch for that.

You can use $wgFoo = [ 'foobar' => true ]. If someone wants to remove it, they can set it to false.

I thought I had left a comment on this ticket earlier, in general I don't like encouraging full overrides. Defaults should be sane, and full overrides end up require needing more maintenance later on. If the defaults aren't sane, that's a different problem. Flat arrays have always been problematic since you need to write some amount of code to be able to remove a single item from them, and we are trying to move away from code in configuration.

EddieGP added a comment.EditedAug 28 2017, 10:51 PM

Okay, I see the rationale for not introducing new flat arrays. I'll just use boolean ones for the task that drove me here then.

But still, we currently need workarounds like this one in the configuration. I'm not convinced that's a good way either.

I understand your comment as a suggestion of declining this task and abandoning the patch (correct me if that's wrong). What's the alternative for dealing with existing flat arrays then? If it is the removal all flat array config variables from all major extensions, is it realistic that all will be refactored in the foreseeable future? If we can't do that, there should probably be a way of dealing with flat arrays - without applying workarounds.

Okay, I see the rationale for not introducing new flat arrays. I'll just use boolean ones for the task that drove me here then.
But still, we currently need workarounds like this one in the configuration. I'm not convinced that's a good way either.
I understand your comment as a suggestion of declining this task and abandoning the patch (correct me if that's wrong). What's the alternative for dealing with existing flat arrays then? If it is the removal all flat array config variables from all major extensions, is it realistic that all will be refactored in the foreseeable future? If we can't do that, there should probably be a way of dealing with flat arrays - without applying workarounds.

Ideally extensions would have migrated during the conversion like what VisualEditor did. Usually we introduce a new config variable that uses key/value, deprecate the old one and map the old one to the new config, and then after a while, deprecate and remove the old config variable.

Change 374042 abandoned by EddieGP:
ExtensionRegistry: Add 'override' merge strategy

https://gerrit.wikimedia.org/r/374042

Is there some function that effectively maps boolean config variables to flat arrays internally in core? So one could have a config variable $wgVar = [ 'foo' => true, 'bar' => false, 'foobar' => true, 'baz' => 'false' ] and then call Something::getBooleanConfigAsFlatArray( 'wgVar' ) which would return [ 'foo', 'foobar' ]? I'm primarily interested in a way to simply iterate over a config variable (which works fine with flat arrays, but not with boolean ones, at least I'm not aware of a really short way) and wrote such a function for https://gerrit.wikimedia.org/r/#/c/380061/. Maybe it'd be worth to have such a helper function in core. That way one could use the boolean config variables and overwrite them in whatever way seems best, but still get config variables as flat arrays when it's necessary/easier at runtime.

You can use array_keys( array_filter( $var ) )

EddieGP closed this task as Declined.Sep 24 2017, 10:25 AM

You can use array_keys( array_filter( $var ) )

Right, thanks! There's no need for some function then. As the preferred solution seems to be not to use flat arrays at all, I already abandoned my patch and am also closing this now.