Skip to content

Reorder command arguments in Pressable_Site_WP_CLI_Command_Run class #100

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.

Sign up for GitHub

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

Open
ecairol wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Reorder command arguments in Pressable_Site_WP_CLI_Command_Run class #100

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions commands/Pressable_Site_WP_CLI_Command_Run.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ protected function configure(): void {
$this->setDescription( 'Runs a given WP-CLI command on a given Pressable site.' )
->setHelp( 'This command allows you to run an arbitrary WP-CLI command on a Pressable site.' );

$this->addArgument( 'wp-cli-command', InputArgument::REQUIRED, 'The WP-CLI command to run.' )
->addArgument( 'site', InputArgument::OPTIONAL, 'The domain or numeric Pressable ID of the site to open the shell to.' );
$this->addArgument( 'site', InputArgument::REQUIRED, 'The domain or numeric Pressable ID of the site to open the shell to.' )
->addArgument( 'wp-cli-command', InputArgument::REQUIRED, 'The WP-CLI command to run.' );
Comment on lines +64 to +65
Copy link

@coderabbitai coderabbitai bot Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Argument reorder + site now REQUIRED — LGTM

Matches the PR objective and fixes the misrouting bug. This is a breaking CLI change; ensure docs/usages are updated.

Run to find and update usages/examples:

🏁 Script executed:

#!/usr/bin/env bash
# Find command invocations/usages to update argument order
rg -nC2 --pretty $'pressable:run-site-wp-cli-command\\b'
rg -nC2 --pretty $'wpcom:run-site-wp-cli-command\\b'

Length of output: 531

Update documentation and examples to reflect new argument order

Search for any external docs or example scripts referencing pressable:run-site-wp-cli-command or wpcom:run-site-wp-cli-command—internal code search only found the command definitions in commands/Pressable_Site_WP_CLI_Command_Run.php and commands/WPCOM_Site_WP_CLI_Command_Run.php. Ensure all user-facing documentation (READMEs, docs/cli guides, sample scripts) are updated so the site argument comes first.

🤖 Prompt for AI Agents 
In commands/Pressable_Site_WP_CLI_Command_Run.php around lines 64-65, the
argument order was changed so the 'site' argument is now first but external
documentation and sample scripts may still show the old order; search the
repository for uses of pressable:run-site-wp-cli-command and
wpcom:run-site-wp-cli-command (README, docs/, sample scripts, CI scripts) and
update any examples and docs to list the arguments with 'site' first followed by
'wp-cli-command', ensuring any displayed command usage, help text, and example
invocations reflect the new order.


$this->addOption( 'multiple', null, InputOption::VALUE_REQUIRED, 'Determines whether the `site` argument is optional or not. Accepted values are `all` or a comma-separated list of site IDs or domains.' )
->addOption( 'skip-output', null, InputOption::VALUE_NONE, 'Skip outputting the response to the console.' );
Comment on lines 67 to 68
Copy link

@coderabbitai coderabbitai bot Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Update --multiple help text to reflect new semantics

It still says it “Determines whether the site argument is optional,” which is no longer true. Clarify that the site argument is ignored when --multiple is used.

Apply:

-        $this->addOption( 'multiple', null, InputOption::VALUE_REQUIRED, 'Determines whether the `site` argument is optional or not. Accepted values are `all` or a comma-separated list of site IDs or domains.' )
+        $this->addOption( 'multiple', null, InputOption::VALUE_REQUIRED, 'Run on multiple sites: `all` or a comma-separated list of site IDs/domains. When set, the single `site` argument is ignored.' )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$this->addOption( 'multiple', null, InputOption::VALUE_REQUIRED, 'Determines whether the `site` argument is optional or not. Accepted values are `all` or a comma-separated list of site IDs or domains.' )
->addOption( 'skip-output', null, InputOption::VALUE_NONE, 'Skip outputting the response to the console.' );
$this->addOption( 'multiple', null, InputOption::VALUE_REQUIRED, 'Run on multiple sites: `all` or a comma-separated list of site IDs/domains. When set, the single `site` argument is ignored.' )
->addOption( 'skip-output', null, InputOption::VALUE_NONE, 'Skip outputting the response to the console.' );
🤖 Prompt for AI Agents 
In commands/Pressable_Site_WP_CLI_Command_Run.php around lines 67 to 68, the
--multiple option help text incorrectly states it "Determines whether the `site`
argument is optional"; update the option description to clarify that when
--multiple is used the `site` argument is ignored and to describe accepted
values (e.g. `all` or a comma-separated list of site IDs or domains), so the new
help text explicitly says the site argument is ignored when --multiple is
provided and lists the valid values.

Expand Down