Use has_config() in get_config() to prevent warnings on null values#5880
Use has_config() in get_config() to prevent warnings on null values#5880danielbachhuber merged 3 commits intowp-cli:mainfrom
Conversation
…f equivelent isset() which throws a warning for null values. A null value can occur if the option is present, but without a value. Certain config values, such as --url, default to null causing has_config() to be true, while throwing a warning in get_config(). This change provides consistency between has_config() and get_config().
|
I was unsure whether the test should go in test-configurator.php, or another file -- I don't see any tests for has/get_config currently. I am also unsure whether it is safe for downstream tests to run |
danielbachhuber
left a comment
There was a problem hiding this comment.
I was unsure whether the test should go in
test-configurator.php, or another file -- I don't see any tests for has/get_config currently. I am also unsure whether it is safe for downstream tests to run$runner->init_config();, please provide feedback and I can yank the test, or amend.
I think what you have is fine. The tests pass 😁
I'm having a little bit of a hard time grokking the nature of the change, though. Can you spell it out for me?
Also, I'd be curious to hear how you ran across this.
There's probably technically a back compat break here, but it's probably fine to make in practice.
|
Hi @danielbachhuber! We had written a script for both single and multisite installs which allowed overriding the site URL with WP-CLI: When tested on a single-site, we quickly saw: After reviewing the code I felt the two functions should behave consistently. I could technically live with both using In #5353, I noted that it is perhaps a different bug in that |
|
@dougaxe1 Thanks for the explanation! Seems reasonable 😊 |
In get_config(), use has_config() to test for key existence instead of equivalent isset() which throws a warning for null values.
A null value can occur if the option is present, but without a value. Certain config values, such as --url, default to null causing has_config() to be true, while throwing a warning in get_config(). This change provides consistency between has_config() and get_config().
Fixes #5353.