Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Same as #2085867: Add a default parameter to keyvalue get() method, but for config. So we would have things like \Drupal::config('my_settings')->get('some_key', 'default value');
Comment | File | Size | Author |
---|---|---|---|
#3 | 2090751-3.patch | 2.39 KB | damiankloip |
#3 | interdiff-2090751-3.txt | 1.48 KB | damiankloip |
#1 | d8.config-get-default.patch | 2.35 KB | damiankloip |
Comments
Comment #1
damiankloip CreditAttribution: damiankloip commentedComment #3
damiankloip CreditAttribution: damiankloip commentedHmm, tis ok locally. Adding another assertion to test get() without any key too.
Comment #4
BerdirWhy is this needed?
Comment #6
damiankloip CreditAttribution: damiankloip commentedTHis has not good a good reception, Let's just close it.
Comment #7
xjmComment #8
ianthomas_ukReopening for the discussion of how to handle defaults for config, on the assumption that defaults for state will be passed as a second parameter.
At the moment, defaults are often set using :?
This will creates confusion for developers between state and config, as they will use different methods
I believe this is also an unintentional change from variable_get in 7.x which fell back to the default using isset(). The PHP ternary operator uses empty(), which means you can't have a boolean default to TRUE.
Both of these could be fixed by enforcing schemas that supplied defaults (so defaults would never be set inline), but I don't think that's currently the plan.
Comment #9
tim.plunkettLet's use real examples, because none of those are legitimate.
Those are all of the places we use a ternary with config().
1) This is a test, and a lazy one at that.
2) This already has a @todo to remove it
3) This is also a test
4) This is a bug, being fixed in #2053461: Node type settings such as published state, promoted state, create revision and author information cannot be turned off
5) This is in system_update_8046() for enabling the views module :)
6) This is cheating, it is casting TRUE to an empty string for the description
7) This is only used during updating (in maintenance mode!)
Every time you call config you should be sure that it exists already. Defaults must be provided. Anywhere we need this pattern, or would need a default parameter, is a bug or a hack.
Comment #10
alexpottWhat is said in #9 is correct doing this would be a massive step backwards for CMI.
Comment #11
ianthomas_ukAgreed. The confusion came because there are still places where defaults are supplied inline, and the documentation is not currently clear enough about how the configuration system should be used, particularly when the config key will be dynamic (e.g. adding additional configuration per-content type). I plan to improve the docs after I've learnt how it all works through #2102521: Finish converting menu.module to CMI
Comment #12
alexpottAnd lets get rid of 3 of those ternaries see #2112233: Remove unnecessary ternaries in around config()->get()