-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Increase visibility of printKeysAndValues() method #9089
Conversation
$expected .= ' [-l] Enable only locator caching.' . PHP_EOL; | ||
$expected .= ' [-d] Disable config and locator caching.' . PHP_EOL; | ||
|
||
$this->assertSame($this->getStreamFilterBuffer(), $expected); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method outputs the options (format like [-c]
) and the values (descriptions).
It seems it is specialized only for command option output.
So the name printKeysAndValues()
is too general. I think more specific name is better.
E.g., printOptionsDescription()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. The test code is not correct.
The method is now used in promptByKey()
. It is used to display options for user input.
It is not for command option.
Then, printKeysAndValues()
is not bad? Any better method name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
printOptionsDescription()
isn't too bad, but the name is a bit long.
how about printOptionsList()
?
@kenjis so, this PR doesn't need a test? |
Yes, it needs test code. |
The method seems to be for only For example, in help messages, we use the following format.
|
If so, i think we should add a new one public function to CLI class to achive this purpose. i believe it very useful to improve our command. Especially when we have a lot of options, implementing --help or -h flags would significantly enhance user experience and efficiency. |
$ ./spark optimize --help
CodeIgniter v4.5.4 Command Line Tool - Server Time: 2024-07-31 20:57:11 UTC+00:00
Usage:
optimize
Description:
Optimize for production. |
Hha, shame on me. I didn't expect that to exist! 😅 Thanks for letting me know. |
Description
This PR makes
printKeysAndValues()
public so we can use it anywhere.Why this needs to be changed:
I have a use case where I need to display the contents of the
$options
property in a command file.It's easier to reuse the existing
printKeysAndValues()
than make a new function with similar behavior. This will help us show more details in our commands.For example: if you type
spark optimize -h
or just some random stuff likespark optimize asdf
, it'll either show you all the possible options or tell you there's a problem and then show you all the options anyway.Checklist: