-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Fix visibility of whitespace in config output #18527
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
base: master
Are you sure you want to change the base?
Conversation
When a config var has whitespace (especially trailing whitespace) it is hard to see. This commit wraps the values (if they exist) in backticks so the difference is visually observable: Before: ``` $ export PHP_INI_SCAN_DIR="/opt/homebrew/etc/php/8.4/conf.d " $ ./sapi/cli/php --ini Configuration File (php.ini) Path: /usr/local/lib Loaded Configuration File: /opt/homebrew/etc/php/8.4/conf.d Scan for additional .ini files in: (none) Additional .ini files parsed: (none) ``` > Note > The above output has trailing whitespace that is not visible, you can see it if you copy it into an editor: After: ``` $ ./sapi/cli/php --ini Configuration File (php.ini) Path: `/usr/local/lib` Loaded Configuration File: `/opt/homebrew/etc/php/8.4/conf.d ` Scan for additional .ini files in: (none) Additional .ini files parsed: (none) ``` Above the whitespace is visible `/opt/homebrew/etc/php/8.4/conf.d `. Close php#18390
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.
Thank you for the PR. I think I would prefer double quotes as the indicator, as this would make the entire value copy and pasteable into a terminal without causing issues, i.e. I can just use the value and run vim "whatever I copied"
to view a config file.
From a suggested comment php#18527 (review). Before: ``` $ ./sapi/cli/php --ini Configuration File (php.ini) Path: `/usr/local/lib` Loaded Configuration File: `/opt/homebrew/etc/php/8.4/conf.d ` Scan for additional .ini files in: (none) Additional .ini files parsed: (none) ``` After: ``` $ ./sapi/cli/php --ini Configuration File (php.ini) Path: "/usr/local/lib" Loaded Configuration File: "/opt/homebrew/etc/php/8.4/conf.d " Scan for additional .ini files in: (none) Additional .ini files parsed: (none) ```
@TimWolla seems good. I initially picked a backtick due to code literal format for markdown I'll generally defer character choice as a style decision for primary project maintainers/contributors. Updated with a new commit and description to match. |
When a config var has whitespace (especially trailing whitespace) it is hard to see. This commit wraps the values (if they exist) in a character so the difference is visually observable:
Before:
Note
The above output has trailing whitespace that is not visible, you can see it if you copy it into an editor.
After:
Above the whitespace is now visible
/opt/homebrew/etc/php/8.4/conf.d
.Close #18390