t4944: Use printf %q for wp_args logging to handle spaces#4950
t4944: Use printf %q for wp_args logging to handle spaces#4950alex-solovyev wants to merge 2 commits intomainfrom
Conversation
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 enhances the debugging experience for WP-CLI commands by ensuring that arguments, especially those containing spaces, are logged with proper shell quoting. This change makes the logged commands directly usable and prevents misinterpretation of complex arguments during troubleshooting, leading to more accurate and unambiguous debug output. 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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughUpdated log message formatting in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🔍 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:43:39 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
The pull request correctly addresses an issue with logging command arguments that contain spaces by using printf %q. This makes the debug output unambiguous. I've suggested a small improvement to remove a trailing space from the log output for better formatting and efficiency, aligning with the practice of using pure Bash parameter expansion for string manipulations to avoid subshells.
.agents/scripts/wp-helper.sh
Outdated
| site_type=$(echo "$site_config" | jq -r '.type') | ||
|
|
||
| print_info "Running on $site_name ($site_type): wp ${wp_args[*]}" >&2 | ||
| print_info "Running on $site_name ($site_type): wp $(printf '%q ' "${wp_args[@]}")" >&2 |
There was a problem hiding this comment.
While using printf '%q ' is a great way to handle arguments with spaces, it introduces a trailing space in the log message. To produce a cleaner log output without the trailing space, you could store the result in a variable and then use shell parameter expansion to trim the last character. This is also slightly more efficient as it avoids forking an external process like sed.
| print_info "Running on $site_name ($site_type): wp $(printf '%q ' "${wp_args[@]}")" >&2 | |
| local args_str | |
| args_str=$(printf '%q ' "${wp_args[@]}") | |
| print_info "Running on $site_name ($site_type): wp ${args_str% }" >&2 |
References
- In shell scripts, use pure Bash parameter expansion for string manipulations to avoid spawning subshells and improve performance.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.agents/scripts/wp-helper.sh (2)
447-447: Same pattern needs updating inrun_on_all.This is the third location using
${wp_args[*]}for logging. Apply the same fix for consistency with line 383:♻️ Proposed fix
- print_info "Command: wp ${wp_args[*]}" + print_info "Command: wp $(printf '%q ' "${wp_args[@]}")"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/wp-helper.sh at line 447, In run_on_all update the logging to use the array-safe expansion: replace the print_info call that currently uses ${wp_args[*]} with the quoted array form "${wp_args[@]}" so individual wp_args elements are preserved; specifically change the print_info invocation that references wp_args to use "${wp_args[@]}" (symbols: run_on_all, wp_args, print_info).
404-404: Same ambiguity issue exists here—apply consistent fix.This line still uses
${wp_args[*]}, which flattens arguments with spaces into ambiguous output (the exact issue this PR addresses at line 383). For consistent, unambiguous logging across all functions:♻️ Proposed fix
- print_info "Command: wp ${wp_args[*]}" + print_info "Command: wp $(printf '%q ' "${wp_args[@]}")"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/wp-helper.sh at line 404, The log uses the ambiguous expansion "${wp_args[*]}"; change logging to produce an unambiguous, safely quoted representation of the arguments instead of using wp_args[*]. For example, build a quoted string from the array with printf '%q ' on "${wp_args[@]}" (or an equivalent loop) and pass that result to print_info (replace the print_info call that references wp_args[*] to use the safe_quoted variable), so use "${wp_args[@]}" with quoting via printf '%q' to preserve argument boundaries and spaces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.agents/scripts/wp-helper.sh:
- Line 447: In run_on_all update the logging to use the array-safe expansion:
replace the print_info call that currently uses ${wp_args[*]} with the quoted
array form "${wp_args[@]}" so individual wp_args elements are preserved;
specifically change the print_info invocation that references wp_args to use
"${wp_args[@]}" (symbols: run_on_all, wp_args, print_info).
- Line 404: The log uses the ambiguous expansion "${wp_args[*]}"; change logging
to produce an unambiguous, safely quoted representation of the arguments instead
of using wp_args[*]. For example, build a quoted string from the array with
printf '%q ' on "${wp_args[@]}" (or an equivalent loop) and pass that result to
print_info (replace the print_info call that references wp_args[*] to use the
safe_quoted variable), so use "${wp_args[@]}" with quoting via printf '%q' to
preserve argument boundaries and spaces.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 524added-3ce7-4abb-9b93-01a9b4c5a871
📒 Files selected for processing (1)
.agents/scripts/wp-helper.sh
…lper.sh Address SonarCloud Maintainability C rating by extracting inline command substitution to a local variable. Apply the same safe-quoting pattern to all three wp_args logging sites (run_wp_command, run_on_category, run_on_all) for consistency. Trim trailing space from printf '%q ' output using parameter expansion per Gemini review suggestion.
|
Dispatching worker to address review suggestions.
|
🔍 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:51:08 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
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.
|
Addressed the Gemini Code Assist suggestion in PR #4953. Change: Replaced the subshell-in-string pattern Also extended the fix to the two remaining Branch: |
…in wp_args log (#4953) * fix: use printf '%q' for wp_args logging to handle spaces in arguments Closes #4944 * fix: use shell parameter expansion to trim trailing space in wp_args log 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. --------- Co-authored-by: Alexey <1556417+alex-solovyev@users.noreply.github.com>
|
Superseded by PR #4953 which addresses the Gemini Code Assist review suggestion (trailing space fix using shell parameter expansion). Closing this original PR. |
2 participants
Summary
${wp_args[*]}with$(printf '%q ' "${wp_args[@]}")inwp-helper.sh:383so arguments containing spaces are logged with proper quoting, making debug output accurate and unambiguous.Closes #4944
Summary by CodeRabbit