Properly create missing wp cli config file when needed.#6062
Merged
swissspidy merged 6 commits intomainfrom Mar 14, 2025
Merged
Properly create missing wp cli config file when needed.#6062swissspidy merged 6 commits intomainfrom
swissspidy merged 6 commits intomainfrom
Conversation
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
Our default config file includes a path that also might not exist. Touch can't create paths, so this silently fails in the one situation you want it to work (when the user hasn't already created the folder). Instead we check if the directory exists first and if not create it. Then we use the build in touch rather than running an external command.
The DocBlock for this function says this should return sring or false but find_file_upward actually returns null when no file is found. This was causing a problem later on because you can't give Spyc::YAMLLoad() a null variable. ``` PHP 15. Mustangostang\Spyc::YAMLLoad($input = NULL) /Users/isla/source/wp-cli-dev/wp-cli/php/commands/src/CLI_Alias_Command.php:358 PHP 16. Mustangostang\Spyc->_load($input = NULL) /Users/isla/source/wp-cli-dev/spyc/src/Spyc.php:118 PHP 17. Mustangostang\Spyc->loadFromSource($input = NULL) /Users/isla/source/wp-cli-dev/spyc/src/Spyc.php:440 PHP 18. Mustangostang\Spyc->loadFromString($input = NULL) /Users/isla/source/wp-cli-dev/spyc/src/Spyc.php:512 PHP 19. explode($separator = '\n', $string = NULL) /Users/isla/source/wp-cli-dev/spyc/src/Spyc.php:516 Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in /Users/isla/source/wp-cli-dev/spyc/src/Spyc.php on line 516 ```
Also fix logging
Need to add an extra debug statement when creating a config file because the existing debug details get overwritten and it only prints the last one
To see if this solves current CI problems
mrsdizzie
force-pushed
the
fix-missing-config
branch
from
3fc5d8d to
172ac4f
Compare
swissspidy
approved these changes
Mar 14, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While looking into #6053 I saw two bugs:
The first is that we called
touchto create a config file if it wasn't present. That doesn't work because touch can't create the parent folders when they don't exist.The only user of this code is the
alias addcommand which I then saw had another bug. The function for getting the path of a project config file returnednullwhen it couldn't find one instead offalseas the docblock says it should. This caused a warning later on: