From 3e9c148b63aeeb089e155ef5c1e070fb15eda00d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Z=C3=BClke?= Date: Fri, 21 Jul 2023 11:29:38 +0200 Subject: [PATCH] Fix trailing whitespace in 'composer show -N' (#11536) The name column was always padded to maximum width, even if no other columns were printed. This makes it difficult to use the output e.g. in pipelines. Fixed for all possible columns, and with tests for two cases (regular show and show outdated). --- src/Composer/Command/ShowCommand.php | 21 ++++++---- .../Composer/Test/Command/ShowCommandTest.php | 41 +++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/Composer/Command/ShowCommand.php b/src/Composer/Command/ShowCommand.php index 50fd058aa..ea5a9db3b 100644 --- a/src/Composer/Command/ShowCommand.php +++ b/src/Composer/Command/ShowCommand.php @@ -442,6 +442,10 @@ EOT $exitCode = 0; $viewData = []; $viewMetaData = []; + + $writeVersion = false; + $writeDescription = false; + foreach (['platform' => true, 'locked' => true, 'available' => false, 'installed' => true] as $type => $showVersion) { if (isset($packages[$type])) { ksort($packages[$type]); @@ -616,14 +620,14 @@ EOT $io->writeError(''); $io->writeError('Direct dependencies required in composer.json:'); if (\count($directDeps) > 0) { - $this->printPackages($io, $directDeps, $indent, $versionFits, $latestFits, $descriptionFits, $width, $versionLength, $nameLength, $latestLength); + $this->printPackages($io, $directDeps, $indent, $writeVersion && $versionFits, $latestFits, $writeDescription && $descriptionFits, $width, $versionLength, $nameLength, $latestLength); } else { $io->writeError('Everything up to date'); } $io->writeError(''); $io->writeError('Transitive dependencies not required in composer.json:'); if (\count($transitiveDeps) > 0) { - $this->printPackages($io, $transitiveDeps, $indent, $versionFits, $latestFits, $descriptionFits, $width, $versionLength, $nameLength, $latestLength); + $this->printPackages($io, $transitiveDeps, $indent, $writeVersion && $versionFits, $latestFits, $writeDescription && $descriptionFits, $width, $versionLength, $nameLength, $latestLength); } else { $io->writeError('Everything up to date'); } @@ -631,7 +635,7 @@ EOT if ($writeLatest && \count($packages) === 0) { $io->writeError('All your direct dependencies are up to date'); } else { - $this->printPackages($io, $packages, $indent, $versionFits, $latestFits, $descriptionFits, $width, $versionLength, $nameLength, $latestLength); + $this->printPackages($io, $packages, $indent, $writeVersion && $versionFits, $writeLatest && $latestFits, $writeDescription && $descriptionFits, $width, $versionLength, $nameLength, $latestLength); } } @@ -649,15 +653,18 @@ EOT */ private function printPackages(IOInterface $io, array $packages, string $indent, bool $writeVersion, bool $writeLatest, bool $writeDescription, int $width, int $versionLength, int $nameLength, int $latestLength): void { + $padName = $writeVersion || $writeLatest || $writeDescription; + $padVersion = $writeLatest || $writeDescription; + $padLatest = $writeDescription; foreach ($packages as $package) { $link = $package['source'] ?? $package['homepage'] ?? ''; if ($link !== '') { - $io->write($indent . ''.$package['name'].''. str_repeat(' ', $nameLength - strlen($package['name'])), false); + $io->write($indent . ''.$package['name'].''. str_repeat(' ', ($padName ? $nameLength - strlen($package['name']) : 0)), false); } else { - $io->write($indent . str_pad($package['name'], $nameLength, ' '), false); + $io->write($indent . str_pad($package['name'], ($padName ? $nameLength : 0), ' '), false); } if (isset($package['version']) && $writeVersion) { - $io->write(' ' . str_pad($package['version'], $versionLength, ' '), false); + $io->write(' ' . str_pad($package['version'], ($padVersion ? $versionLength : 0), ' '), false); } if (isset($package['latest']) && isset($package['latest-status']) && $writeLatest) { $latestVersion = $package['latest']; @@ -666,7 +673,7 @@ EOT if (!$io->isDecorated()) { $latestVersion = str_replace(['up-to-date', 'semver-safe-update', 'update-possible'], ['=', '!', '~'], $updateStatus) . ' ' . $latestVersion; } - $io->write(' <' . $style . '>' . str_pad($latestVersion, $latestLength, ' ') . '', false); + $io->write(' <' . $style . '>' . str_pad($latestVersion, ($padLatest ? $latestLength : 0), ' ') . '', false); } if (isset($package['description']) && $writeDescription) { $description = strtok($package['description'], "\r\n"); diff --git a/tests/Composer/Test/Command/ShowCommandTest.php b/tests/Composer/Test/Command/ShowCommandTest.php index 4dbf25872..c3b719618 100644 --- a/tests/Composer/Test/Command/ShowCommandTest.php +++ b/tests/Composer/Test/Command/ShowCommandTest.php @@ -404,4 +404,45 @@ available: installed: vendor/installed 2.0.0 description of installed package', $output); } + + public function testNameOnlyPrintsNoTrailingWhitespace(): void + { + $this->initTempComposer([ + 'repositories' => [ + 'packages' => [ + 'type' => 'package', + 'package' => [ + // CAUTION: package names matter - output is sorted, and we want shorter before longer ones + ['name' => 'vendor/apackage', 'description' => 'generic description', 'version' => '1.0.0'], + ['name' => 'vendor/apackage', 'description' => 'generic description', 'version' => '1.1.0'], + ['name' => 'vendor/longpackagename', 'description' => 'generic description', 'version' => '1.0.0'], + ['name' => 'vendor/longpackagename', 'description' => 'generic description', 'version' => '1.1.0'], + ['name' => 'vendor/somepackage', 'description' => 'generic description', 'version' => '1.0.0'], + ], + ], + ], + ]); + + $this->createInstalledJson([ + self::getPackage('vendor/apackage', '1.0.0'), + self::getPackage('vendor/longpackagename', '1.0.0'), + self::getPackage('vendor/somepackage', '1.0.0'), + ]); + + $appTester = $this->getApplicationTester(); + $appTester->run(['command' => 'show', '-N' => true]); + self::assertSame( +'vendor/apackage +vendor/longpackagename +vendor/somepackage', trim($appTester->getDisplay(true))); // trim() is fine here, but see CAUTION above + + $appTester = $this->getApplicationTester(); + $appTester->run(['command' => 'show', '--outdated' => true, '-N' => true]); + self::assertSame( +'Legend: +! patch or minor release available - update recommended +~ major release available - update possible +vendor/apackage +vendor/longpackagename', trim($appTester->getDisplay(true))); // trim() is fine here, but see CAUTION above + } }