This is a function to merge 2 arrays into one
array by overruling the values of the first array if the
key is identical.
This is e.g. usefull to merge a global configuration file
with a stage specific configuration.
Differential D9135
Add function to merge 2 arrays by overruling the first sascha-egerer on May 15 2014, 3:02 PM. Authored by Tags None Referenced Files
Subscribers
Details
This is a function to merge 2 arrays into one This is e.g. usefull to merge a global configuration file Check and execute unit tests
Diff Detail
Event TimelineComment Actions
Where do you want to use this specifically? I worry this won't be sufficient in the general case. If both configuration files have lists, sometimes those lists should be combined instead of overwritten, and other times they should be overwritten entirely. For example, if you're merging .arclint files like this: { "exclude" : ["(^globalstuff/)"] } And: { "exclude" : ["(^localstuff/)"] } The best way to merge these files is probably to produce ["(^globalstuff/)", "(^localstuff/)"], but this won't do that. Or, suppose you're merging .arcconfig files like this: { "load": ["a", "b"], } And: { "load" : ["z"], } ...doesn't that produce ["z", "b"], a nonsensical/bizarre result? Comment Actions Yes, it would not work with arrays that have no or numerical keys like in you example. We have a global configuration file for phabricator to configure stuff. That is located in conf/custom/global.conf.php. For each stage of our deployment we have a seperate config file to adjust settings like conf/custom/integration.conf.php which is loaded by the envirement. global.conf.php return array( 'phabricator.show-beta-applications' => true, 'phabricator.uninstalled-applications' => array( 'PhabricatorApplicationUIExamples' => false, 'PhabricatorApplicationXHProf' => false ) ); We want to load the global.conf.php and overrule some settings for a stage. Currently we use the + operator to merge the arrays but this will not return the expected result here. production.conf.php return array( 'phabricator.show-beta-applications' => false, 'phabricator.uninstalled-applications' => array( 'PhabricatorApplicationUIExamples' => true, ) ) + phabricator_read_config_file('custom/global.conf.php'); Comment Actions I've just run the unit test cases against +, array_merge, array_merge_recursive and array_merge_recursive_overrule to show the different result. P1142 Comment Actions I think this implementation is correct, but we don't have any real use cases in libphutil/arcanist/phabricator, because anywhere we might want to merge configuration can be better covered by rules which handle the cases above. So, the code looks fine, but I don't want to put it in the upstream without use cases motivating it. Comment Actions As already said in IRC I'll implement this function with a custom script and abondon this revision. |