fix(wp-helper): use shell parameter expansion to trim trailing space in wp_args log#4953
fix(wp-helper): use shell parameter expansion to trim trailing space in wp_args log#4953marcusquinn merged 2 commits intomainfrom
Conversation
Replace subshell-in-string $(printf '%q ' ...) with a two-step pattern:
local args_str; args_str=$(printf '%q ' "${wp_args[@]}")
... ${args_str% }
This avoids spawning an extra subshell inside the double-quoted string and
uses pure shell parameter expansion (${args_str% }) to strip the trailing
space left by printf '%q '. Applied to all three logging sites:
run_wp_command (line 383), run_on_category (line 404), run_on_all (line 447).
Addresses Gemini Code Assist suggestion on PR #4950.
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the logging of Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Mar 15 18:52:58 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue with logging shell arguments by replacing the unsafe ${wp_args[*]} with a more robust method using printf '%q '. This improves correctness and readability, and the changes are applied consistently. I have a few suggestions to further improve the implementation by using printf -v to avoid creating subshells, which aligns with the performance goals stated in the pull request description and with established repository rules regarding subshell avoidance for performance.
|
|
||
| print_info "Running on $site_name ($site_type): wp ${wp_args[*]}" >&2 | ||
| local args_str | ||
| args_str=$(printf '%q ' "${wp_args[@]}") |
There was a problem hiding this comment.
Using command substitution $(...) creates a subshell. To make this more efficient and align with the goal of avoiding subshells, you can use printf -v. This bash feature assigns the output directly to the variable without creating a new process.
| args_str=$(printf '%q ' "${wp_args[@]}") | |
| printf -v args_str '%q ' "${wp_args[@]}" |
References
- In shell scripts, avoid spawning subshells for string manipulations to improve performance. Using
printf -valigns with this principle by assigning output directly to a variable without creating a new process, similar to using pure Bash parameter expansion.
| print_info "Running on all sites in category: $category" | ||
| print_info "Command: wp ${wp_args[*]}" | ||
| local args_str | ||
| args_str=$(printf '%q ' "${wp_args[@]}") |
There was a problem hiding this comment.
Using command substitution $(...) creates a subshell. To make this more efficient and align with the goal of avoiding subshells, you can use printf -v. This bash feature assigns the output directly to the variable without creating a new process.
| args_str=$(printf '%q ' "${wp_args[@]}") | |
| printf -v args_str '%q ' "${wp_args[@]}" |
References
- In shell scripts, avoid spawning subshells for string manipulations to improve performance. Using
printf -valigns with this principle by assigning output directly to a variable without creating a new process, similar to using pure Bash parameter expansion.
| print_info "Running on ALL sites" | ||
| print_info "Command: wp ${wp_args[*]}" | ||
| local args_str | ||
| args_str=$(printf '%q ' "${wp_args[@]}") |
There was a problem hiding this comment.
Using command substitution $(...) creates a subshell. To make this more efficient and align with the goal of avoiding subshells, you can use printf -v. This bash feature assigns the output directly to the variable without creating a new process.
| args_str=$(printf '%q ' "${wp_args[@]}") | |
| printf -v args_str '%q ' "${wp_args[@]}" |
References
- In shell scripts, avoid spawning subshells for string manipulations to improve performance. Using
printf -valigns with this principle by assigning output directly to a variable without creating a new process, similar to using pure Bash parameter expansion.
2 participants
Summary
Addresses Gemini Code Assist review suggestion on PR #4950.
$(printf '%q ' "${wp_args[@]}")with a two-step approach using shell parameter expansion${args_str% }to trim the trailing space left byprintf '%q 'wp-helper.sh:run_wp_command(line 383),run_on_category(line 404), andrun_on_all(line 447)${wp_args[*]}expansion (lines 404 and 447), which were flagged by CodeRabbit on PR t4944: Use printf %q for wp_args logging to handle spaces #4950Why
Using
${args_str% }instead of a subshell inside a double-quoted string:localdeclaration and assignment on separate lines (ShellCheck-clean)Related
bugfix/t4944-wp-args-logging) to cover all three logging sites