From 4231f69249041777294e2c27262add3d07a3b5fc Mon Sep 17 00:00:00 2001
From: AnrDaemon <anrdaemon@yandex.ru>
Date: Sat, 4 Aug 2018 00:29:31 +0300
Subject: [PATCH] Improved help rendering.
Improved synopsis rendering.
Improved operands rendering.
Reduced clutter.
Reduced the number of assumptions in rendering.
Removed duplicates.
Fixed failing tests.
Seriously, why do you use PHP_EOL in message and then suddenly compare to plain \n in test?
---
src/Help.php | 35 ++++++++++++++++------------------
test/Operands/HelpTest.php | 14 +++++++-------
test/Operands/StrictTest.php | 2 +-
test/Translator/CommonTest.php | 2 +-
4 files changed, 25 insertions(+), 28 deletions(-)
diff --git a/src/Help.php b/src/Help.php
index d8a9f13..6610aa8 100644
--- a/src/Help.php
+++ b/src/Help.php
@@ -175,7 +175,7 @@ class Help implements HelpInterface
protected function renderUsage()
{
return $this->getText('usage-title') .
- $this->getOpt->get(GetOpt::SETTING_SCRIPT_NAME) . ' ' .
+ $this->getOpt->get(GetOpt::SETTING_SCRIPT_NAME) .
$this->renderUsageCommand() .
$this->renderUsageOptions() .
$this->renderUsageOperands() . PHP_EOL . PHP_EOL .
@@ -189,8 +189,8 @@ class Help implements HelpInterface
$hasDescriptions = false;
foreach ($this->getOpt->getOperandObjects() as $operand) {
$definition = $this->surround($operand->getName(), $this->texts['placeholder']);
- if (!$operand->isRequired()) {
- $definition = $this->surround($definition, $this->texts['optional']);
+ if ($operand->isMultiple()) {
+ $definition .= $this->texts['multiple'];
}
if (strlen($definition) > $definitionWidth) {
@@ -261,47 +261,44 @@ class Help implements HelpInterface
protected function renderUsageCommand()
{
if ($command = $this->getOpt->getCommand()) {
- return $command->getName() . ' ';
+ return ' ' . $command->getName();
} elseif ($this->getOpt->hasCommands()) {
- return $this->surround($this->getText('usage-command'), $this->texts['placeholder']) . ' ';
+ return ' ' . $this->surround($this->getText('usage-command'), $this->texts['placeholder']);
}
return '';
}
-
+
protected function renderUsageOptions()
{
if ($this->getOpt->hasOptions() || !$this->getOpt->get(GetOpt::SETTING_STRICT_OPTIONS)) {
- return $this->surround($this->getText('usage-options'), $this->texts['optional']) . ' ';
+ return ' ' . $this->surround($this->getText('usage-options'), $this->texts['optional']);
}
}
-
+
protected function renderUsageOperands()
{
$usage = '';
-
+
$lastOperandMultiple = false;
if ($this->getOpt->hasOperands()) {
foreach ($this->getOpt->getOperandObjects() as $operand) {
$name = $this->surround($operand->getName(), $this->texts['placeholder']);
- if (!$operand->isRequired()) {
- $name = $this->surround($name, $this->texts['optional']);
- }
- $usage .= $name . ' ';
if ($operand->isMultiple()) {
- $usage .= $this->surround(
- $this->surround($operand->getName(), $this->texts['placeholder']) . $this->texts['multiple'],
- $this->texts['optional']
- );
+ $name .= $this->texts['multiple'];
$lastOperandMultiple = true;
}
+ if (!$operand->isRequired()) {
+ $name = $this->surround($name, $this->texts['optional']);
+ }
+ $usage .= ' ' . $name;
}
}
if (!$lastOperandMultiple && !$this->getOpt->get(GetOpt::SETTING_STRICT_OPERANDS)) {
- $usage .= $this->surround($this->getText('usage-operands'), $this->texts['optional']);
+ $usage .= ' ' . $this->surround($this->getText('usage-operands'), $this->texts['optional']);
}
-
+
return $usage;
}
diff --git a/test/Operands/HelpTest.php b/test/Operands/HelpTest.php
index 3947e04..8c46421 100644
--- a/test/Operands/HelpTest.php
+++ b/test/Operands/HelpTest.php
@@ -54,14 +54,14 @@ class HelpTest extends TestCase
/** @test */
public function helpTextForMultiple()
{
- $operand = new Operand('op1', Operand::MULTIPLE);
+ $operand = new Operand('op1', Operand::OPTIONAL | Operand::MULTIPLE);
$script = $_SERVER['PHP_SELF'];
$getopt = new GetOpt();
$getopt->addOperand($operand);
self::assertSame(
- 'Usage: ' . $script . ' [<op1>] [<op1>...]' . PHP_EOL . PHP_EOL,
+ 'Usage: ' . $script . ' [<op1>...]' . PHP_EOL . PHP_EOL,
$getopt->getHelpText()
);
}
@@ -76,7 +76,7 @@ class HelpTest extends TestCase
$getopt->addOperand($operand);
self::assertSame(
- 'Usage: ' . $script . ' <op1> [<op1>...]' . PHP_EOL . PHP_EOL,
+ 'Usage: ' . $script . ' <op1>...' . PHP_EOL . PHP_EOL,
$getopt->getHelpText()
);
}
@@ -96,10 +96,10 @@ class HelpTest extends TestCase
);
self::assertSame(
- 'Usage: ' . $script . ' <file> [<destination>] ' . PHP_EOL . PHP_EOL .
+ 'Usage: ' . $script . ' <file> [<destination>]' . PHP_EOL . PHP_EOL .
'Operands:' . PHP_EOL .
- ' <file> The file to copy' . PHP_EOL .
- ' [<destination>] The destination folder (current folder by default)' . PHP_EOL . PHP_EOL,
+ ' <file> The file to copy' . PHP_EOL .
+ ' <destination> The destination folder (current folder by default)' . PHP_EOL . PHP_EOL,
$getOpt->getHelpText()
);
}
@@ -119,7 +119,7 @@ class HelpTest extends TestCase
);
self::assertSame(
- 'Usage: ' . $script . ' <file> [<destination>] ' . PHP_EOL . PHP_EOL,
+ 'Usage: ' . $script . ' <file> [<destination>]' . PHP_EOL . PHP_EOL,
$getOpt->getHelpText([Help::HIDE_OPERANDS => true])
);
}
diff --git a/test/Operands/StrictTest.php b/test/Operands/StrictTest.php
index 75ec2a4..ed66065 100644
--- a/test/Operands/StrictTest.php
+++ b/test/Operands/StrictTest.php
@@ -39,7 +39,7 @@ class StrictTest extends TestCase
$script = $_SERVER['PHP_SELF'];
self::assertSame(
- 'Usage: ' . $script . ' <file> ' . PHP_EOL . PHP_EOL,
+ 'Usage: ' . $script . ' <file>' . PHP_EOL . PHP_EOL,
$getopt->getHelpText()
);
}
diff --git a/test/Translator/CommonTest.php b/test/Translator/CommonTest.php
index d48364b..3207150 100644
--- a/test/Translator/CommonTest.php
+++ b/test/Translator/CommonTest.php
@@ -32,6 +32,6 @@ class CommonTest extends TestCase
$result = $translator->translate('commands-title');
- self::assertSame("Commands:\n", $result);
+ self::assertSame("Commands:" . PHP_EOL, $result);
}
}
--
GitLab