refactor: update Console::run() to allow it be called in command() function#10080
refactor: update Console::run() to allow it be called in command() function#10080paulbalandan wants to merge 4 commits intocodeigniter4:4.8from
Console::run() to allow it be called in command() function#10080Conversation
paulbalandan
added
refactor
paulbalandan
force-pushed
the
refactor-console
branch
from
e06e449 to
a7fdc17
Compare
paulbalandan
force-pushed
the
refactor-console
branch
from
a7fdc17 to
2c1f8cc
Compare
| if (! in_array('--no-header', $tokens, true)) { | ||
| // Don't show the header as it is not needed when running commands from code. | ||
| $tokens[] = '--no-header'; | ||
| } |
There was a problem hiding this comment.
Won't this break calls that use the -- separator? Because --no-header is appended after tokenization, it ends up after the separator and is treated as a positional argument, so the banner is printed and the target command receives an unexpected --no-header argument.
There was a problem hiding this comment.
Thanks. Missed that part.
| $exit = $console->run(); | ||
|
|
||
| return is_int($exit) ? $exit : EXIT_SUCCESS; | ||
| return $console->initialize()->run() ?? EXIT_SUCCESS; |
There was a problem hiding this comment.
Previously, we converted any non-int result to EXIT_SUCCESS. Now, a legacy/custom command returning a bool/string can cause a return-type error.
There was a problem hiding this comment.
Is bool|string allowed before as a return type? As I remember it, Console::run() has a PHPDoc of int|void.
There was a problem hiding this comment.
Well, yes... bool|string were not part of the "contract", which was effectively int|void, but run() is not natively typed, so non-int values were still accepted at runtime. Previously, we handled that defensively by normalizing any non-int to EXIT_SUCCESS. IMHO, if we want to enforce stricter behavior now, we should make that explicit in the docs (and upgrade notes?) and give users time to adjust.
2 participants
Description
This refactors
Console::run()so that it can accept a list of tokens to parse (defaulting to$_SERVER['argv']if empty) whenrun()is called. This consequently allows it to be called inside thecommand()function so that the latter can also enjoy the enhancements to command line parsing.A behavior break that I am seeing is that
command()now receives the$paramsas a merge of arguments and options, instead of the previous how-it-was-parsed order. However, since getting the params passed to a command by this function is not an intended use, I believe its impact is minimal.Checklist: