Skip to content

Commit 60c5779

Browse files
CopilotswissspidyCopilot
authored
Error when scaffold package-readme commands are not loaded (#271)
* Initial plan * Add error check when commands are not found in cmd_dump Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Improve error message to include specific missing command Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Load command file when generating README during scaffold When wp scaffold package creates a new package, it now explicitly loads the command file (hello-world-command.php) before calling package-readme. This ensures the command is registered in WP-CLI so it can be properly documented in the generated README. Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Use launch=true to load wp-cli.yml when scaffolding README Changed package-readme call to use launch=true instead of launch=false. This spawns a new WP-CLI process that automatically loads the wp-cli.yml file from the package directory, which in turn loads the command file. This is cleaner than hardcoding the command filename. Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * partial revert, add another test * first install, then readme * Skip README generation when package installation is skipped When wp scaffold package is called with --skip-install, the package is not installed, so commands are not loaded. This means package-readme would fail with the error check for missing commands. To avoid this, README generation is now skipped when --skip-install is used, unless explicitly overridden with the combination of --skip-install without --skip-readme. Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Use launch=true for package-readme to ensure commands are loaded When scaffold package installs a package and then calls package-readme, the newly installed package needs to be available. Using launch=true spawns a new WP-CLI process where the package installation is fully effective and commands are properly loaded. Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Skip README generation when package installation is skipped When --skip-install is used, the package is not installed, so commands won't be loaded. This would cause package-readme to fail with the missing commands error. Instead, we now skip README generation with a warning message explaining that commands must be loaded for complete documentation. Users who want to skip installation but still generate a README can manually run 'wp scaffold package-readme' after ensuring commands are loaded via wp-cli.yml or package installation. Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Fix path resolution for launch=true and remove warning message - Convert relative package paths to absolute paths before passing to subprocess via launch=true - Remove warning message when --skip-install is used (silently skip README instead) - Fix PHPStan error with optional 'heading' offset by using null coalescing operator This ensures package-readme can find directories when spawned in a new process, and tests don't see unexpected warning messages. Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Restore original order: generate README before package installation Reverted to the original order where README generation happens BEFORE package installation. This avoids issues with Composer's autoloader being modified by package installation and then causing problems when spawning new processes. The flow is now: 1. Scaffold tests 2. Generate README (using wp-cli.yml to load commands) 3. Scaffold GitHub files 4. Install package (if not skipped) This way README generation works with launch=false and --path flag, and doesn't conflict with package installation. Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * fix attempt * Lint fixes * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update src/ScaffoldPackageCommand.php * Update src/ScaffoldPackageCommand.php * Add --force flag to test to overwrite existing README The test "Does not error when commands are specified and present" calls scaffold package which generates a README, then calls scaffold package-readme again. Without --force, it prompts for user input causing the test to fail. Adding --force allows the README to be overwritten without prompting. Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Apply suggestion from @swissspidy --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent f3df83d commit 60c5779

File tree

2 files changed

+120
-51
lines changed

2 files changed

+120
-51
lines changed

‎features/scaffold-package-readme.feature‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,6 @@ Feature: Scaffold a README.md file for an existing package
294294
*This README.md is generated dynamically from the project's codebase
295295
"""
296296

297-
@broken
298297
Scenario: Error when commands are specified but not present
299298
Given an empty directory
300299
And a foo/composer.json file:
@@ -318,6 +317,17 @@ Feature: Scaffold a README.md file for an existing package
318317
"""
319318
And the return code should be 1
320319

320+
Scenario: Does not error when commands are specified and present
321+
Given an empty directory
322+
When I run `wp scaffold package wp-cli/foo --dir=foo`
323+
And I try `composer install -d foo`
324+
And I try `wp scaffold package-readme foo --force`
325+
Then STDERR should not contain:
326+
"""
327+
Error: Missing one or more commands defined in composer.json -> extra -> commands.
328+
"""
329+
And I run `wp package uninstall wp-cli/foo`
330+
321331
Scenario: README for a bundled command
322332
Given an empty directory
323333
And a foo/composer.json file:

‎src/ScaffoldPackageCommand.php‎

Lines changed: 109 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ class ScaffoldPackageCommand {
5555
* ---
5656
* default: ^5.0.0
5757
* ---
58-
5958
* [--skip-tests]
6059
* : Don't generate files for integration testing.
6160
*
@@ -105,6 +104,15 @@ public function package( $args, $assoc_args ) {
105104
}
106105
}
107106

107+
// Convert to absolute path if relative.
108+
if ( ! preg_match( '#^([a-zA-Z]:)?[\\/]#', $package_dir ) ) {
109+
$cwd = getcwd();
110+
if ( false === $cwd ) {
111+
WP_CLI::error( 'Could not determine current working directory.' );
112+
}
113+
$package_dir = $cwd . DIRECTORY_SEPARATOR . $package_dir;
114+
}
115+
108116
if ( empty( $assoc_args['homepage'] ) ) {
109117
$assoc_args['homepage'] = 'https://github.com/' . $assoc_args['name'];
110118
}
@@ -140,21 +148,24 @@ public function package( $args, $assoc_args ) {
140148
WP_CLI::success( "Created package files in {$package_dir}" );
141149
}
142150

143-
$force_flag = $force ? '--force' : '';
144-
if ( ! Utils\get_flag_value( $assoc_args, 'skip-tests' ) ) {
145-
WP_CLI::runcommand( "scaffold package-tests {$package_dir} {$force_flag}", array( 'launch' => false ) );
146-
}
151+
$force_flag = $force ? '--force' : '';
152+
$quoted_package_dir = escapeshellarg( $package_dir );
147153

148-
if ( ! Utils\get_flag_value( $assoc_args, 'skip-readme' ) ) {
149-
WP_CLI::runcommand( "scaffold package-readme {$package_dir} {$force_flag}", array( 'launch' => false ) );
154+
if ( ! Utils\get_flag_value( $assoc_args, 'skip-tests' ) ) {
155+
WP_CLI::runcommand( "scaffold package-tests {$quoted_package_dir} {$force_flag}", array( 'launch' => false ) );
150156
}
151157

152158
if ( ! Utils\get_flag_value( $assoc_args, 'skip-github' ) ) {
153-
WP_CLI::runcommand( "scaffold package-github {$package_dir} {$force_flag}", array( 'launch' => false ) );
159+
WP_CLI::runcommand( "scaffold package-github {$quoted_package_dir} {$force_flag}", array( 'launch' => false ) );
154160
}
155161

156162
if ( ! Utils\get_flag_value( $assoc_args, 'skip-install' ) ) {
157-
WP_CLI::runcommand( "package install {$package_dir}", array( 'launch' => false ) );
163+
Process::create( Utils\esc_cmd( 'composer', 'install', '--working-dir', $package_dir ) )->run();
164+
WP_CLI::runcommand( "package install {$quoted_package_dir}", array( 'launch' => false ) );
165+
}
166+
167+
if ( ! Utils\get_flag_value( $assoc_args, 'skip-readme' ) ) {
168+
WP_CLI::runcommand( "scaffold package-readme {$quoted_package_dir} {$force_flag}", array( 'launch' => false ) );
158169
}
159170

160171
// Display next steps guidance for users.
@@ -327,56 +338,104 @@ public function package_readme( $args, $assoc_args ) {
327338

328339
if ( ! empty( $composer_obj['extra']['commands'] ) ) {
329340
$readme_args['commands'] = [];
330-
$cmd_dump = WP_CLI::runcommand(
341+
342+
// Make relative --require paths absolute, as they will be invalid after chdir().
343+
/**
344+
* @var array{argv: array<string,string>} $GLOBALS
345+
*/
346+
$orig_argv = $GLOBALS['argv'];
347+
$cwd = getcwd();
348+
if ( false === $cwd ) {
349+
WP_CLI::error( 'Could not determine current working directory.' );
350+
}
351+
foreach ( $GLOBALS['argv'] as &$arg ) {
352+
if ( 0 === strpos( $arg, '--require=' ) ) {
353+
$req_path = substr( $arg, 10 );
354+
if ( ! \WP_CLI\Utils\is_path_absolute( $req_path ) ) {
355+
$arg = '--require=' . $cwd . '/' . $req_path;
356+
}
357+
}
358+
}
359+
unset( $arg );
360+
chdir( $package_dir );
361+
362+
$cmd_dump = WP_CLI::runcommand(
331363
'cli cmd-dump',
332364
[
333-
'launch' => false,
334-
'return' => true,
335-
'parse' => 'json',
365+
'launch' => true,
366+
'return' => 'all',
367+
'parse' => false,
368+
'exit_error' => false,
336369
]
337370
);
338-
foreach ( $composer_obj['extra']['commands'] as $command ) {
339-
$bits = explode( ' ', $command );
340-
$parent_command = $cmd_dump;
341-
do {
342-
$cmd_bit = array_shift( $bits );
343-
$found = false;
344-
foreach ( $parent_command['subcommands'] as $subcommand ) {
345-
if ( $subcommand['name'] === $cmd_bit ) {
346-
$parent_command = $subcommand;
347-
$found = true;
348-
break;
349-
}
350-
}
351-
if ( ! $found ) {
352-
$parent_command = false;
353-
}
354-
} while ( $parent_command && $bits );
355371

356-
/* This check doesn't work because of the way the commands are fetched.
357-
* Needs bigger refactor to put this check back in.
358-
if ( empty( $parent_command ) ) {
359-
WP_CLI::error( 'Missing one or more commands defined in composer.json -> extra -> commands.' );
372+
if ( 0 !== $cmd_dump->return_code ) {
373+
$cmd_dump = null;
374+
} else {
375+
$cmd_dump = json_decode( $cmd_dump->stdout, true );
376+
if ( JSON_ERROR_NONE !== json_last_error() ) {
377+
$cmd_dump = null;
360378
}
361-
*/
379+
}
362380

363-
$longdesc = isset( $parent_command['longdesc'] ) ? $parent_command['longdesc'] : '';
364-
$longdesc = (string) preg_replace( '/## GLOBAL PARAMETERS(.+)/s', '', $longdesc );
365-
$longdesc = (string) preg_replace( '/##\s(.+)/', '**$1**', $longdesc );
381+
/**
382+
* @var null|array{name: string, description: string, longdesc: string, hook: string, alias?: string, subcommands: array<array{name: string, description: string, longdesc: string, hook: string}>} $cmd_dump
383+
*/
366384

367-
// definition lists
368-
$longdesc = preg_replace_callback( '/([^\n]+)\n: (.+?)(\n\n(?=\S)|\n*$)/s', [ __CLASS__, 'rewrap_param_desc' ], $longdesc );
385+
chdir( $cwd );
386+
// phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound
387+
$GLOBALS['argv'] = $orig_argv;
388+
foreach ( $composer_obj['extra']['commands'] as $command ) {
389+
$bits = explode( ' ', $command );
390+
$parent_command = $cmd_dump;
369391

370-
$command_data = [
371-
'name' => "wp {$command}",
372-
'shortdesc' => isset( $parent_command['description'] ) ? $parent_command['description'] : '',
373-
'synopsis' => "wp {$command}" . ( empty( $parent_command['subcommands'] ) ? ( isset( $parent_command['synopsis'] ) ? " {$parent_command['synopsis']}" : '' ) : '' ),
374-
'longdesc' => $longdesc,
375-
];
392+
if ( $parent_command ) {
393+
do {
394+
$cmd_bit = array_shift( $bits );
395+
$found = false;
396+
397+
foreach ( $parent_command['subcommands'] ?? [] as $subcommand ) {
398+
if ( $subcommand['name'] === $cmd_bit ) {
399+
$parent_command = $subcommand;
400+
$found = true;
401+
break;
402+
}
403+
}
404+
if ( ! $found ) {
405+
$parent_command = false;
406+
}
407+
} while ( $parent_command && $bits );
408+
}
376409

377-
// Add alias if present.
378-
if ( ! empty( $parent_command['alias'] ) ) {
379-
$command_data['alias'] = $parent_command['alias'];
410+
if ( empty( $parent_command ) ) {
411+
if ( null !== $cmd_dump ) {
412+
WP_CLI::error( 'Missing one or more commands defined in composer.json -> extra -> commands.' );
413+
}
414+
$command_data = [
415+
'name' => "wp {$command}",
416+
'shortdesc' => '',
417+
'synopsis' => '',
418+
'longdesc' => '',
419+
];
420+
} else {
421+
$longdesc = isset( $parent_command['longdesc'] ) ? $parent_command['longdesc'] : '';
422+
$longdesc = (string) preg_replace( '/## GLOBAL PARAMETERS(.+)/s', '', $longdesc );
423+
$longdesc = (string) preg_replace( '/##\s(.+)/', '**$1**', $longdesc );
424+
425+
// definition lists
426+
$longdesc = preg_replace_callback( '/([^\n]+)\n: (.+?)(\n\n(?=\S)|\n*$)/s', [ __CLASS__, 'rewrap_param_desc' ], $longdesc );
427+
428+
$command_data = [
429+
'name' => "wp {$command}",
430+
'shortdesc' => isset( $parent_command['description'] ) ? $parent_command['description'] : '',
431+
'synopsis' => "wp {$command}" . ( empty( $parent_command['subcommands'] ) ? ( isset( $parent_command['synopsis'] ) ? " {$parent_command['synopsis']}" : '' ) : '' ),
432+
'longdesc' => $longdesc,
433+
];
434+
435+
// Add alias if present.
436+
if ( ! empty( $parent_command['alias'] ) ) {
437+
$command_data['alias'] = $parent_command['alias'];
438+
}
380439
}
381440

382441
$readme_args['commands'][] = $command_data;
@@ -460,7 +519,7 @@ public function package_readme( $args, $assoc_args ) {
460519
$readme_args['package_description'] = $value;
461520
} else {
462521
$readme_args['sections'][] = [
463-
'heading' => $section_args['heading'],
522+
'heading' => $section_args['heading'] ?? '',
464523
'body' => $value,
465524
];
466525
}

0 commit comments

Comments
 (0)