From bf76fa547d9f8c6704d99db550b2e58e51ce906d Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 25 Apr 2020 08:52:12 -0700 Subject: [PATCH] Make "arc --help" work again for workflows which haven't updated yet Summary: See . The "--help" flag ends up falling through to the old "arcanist.php", where it becomes lost. Catch it earlier so "arc diff --help" prints diff help, for instance. Test Plan: Ran `arc help diff`, `arc diff --help`, `arc --help diff`, and similar commands for updated workflows; got help. Differential Revision: https://secure.phabricator.com/D21168 --- src/parser/argument/PhutilArgumentParser.php | 33 ++++++++++-- .../workflow/PhutilHelpArgumentWorkflow.php | 52 ------------------- src/runtime/ArcanistRuntime.php | 40 ++++++++++++++ src/toolset/ArcanistWorkflowArgument.php | 5 ++ src/toolset/workflow/ArcanistHelpWorkflow.php | 3 +- 5 files changed, 74 insertions(+), 59 deletions(-) diff --git a/src/parser/argument/PhutilArgumentParser.php b/src/parser/argument/PhutilArgumentParser.php index 81f86cde..6fe5bc66 100644 --- a/src/parser/argument/PhutilArgumentParser.php +++ b/src/parser/argument/PhutilArgumentParser.php @@ -77,6 +77,7 @@ final class PhutilArgumentParser extends Phobject { private $tagline; private $synopsis; private $workflows; + private $helpWorkflows; private $showHelp; private $requireArgumentTerminator = false; private $sawTerminator = false; @@ -449,11 +450,13 @@ final class PhutilArgumentParser extends Phobject { $flow = $corrected; } else { - $this->raiseUnknownWorkflow($flow, $corrected); + if (!$this->showHelp) { + $this->raiseUnknownWorkflow($flow, $corrected); + } } } - $workflow = $this->workflows[$flow]; + $workflow = idx($this->workflows, $flow); if ($this->showHelp) { // Make "cmd flow --help" behave like "cmd help flow", not "cmd help". @@ -471,6 +474,10 @@ final class PhutilArgumentParser extends Phobject { } } + if (!$workflow) { + $this->raiseUnknownWorkflow($flow, $corrected); + } + $this->argv = array_values($argv); if ($workflow->shouldParsePartial()) { @@ -633,6 +640,12 @@ final class PhutilArgumentParser extends Phobject { return $this; } + public function setHelpWorkflows(array $help_workflows) { + $help_workflows = mpull($help_workflows, null, 'getName'); + $this->helpWorkflows = $help_workflows; + return $this; + } + public function getWorkflows() { return $this->workflows; } @@ -684,11 +697,16 @@ final class PhutilArgumentParser extends Phobject { $out[] = null; } - if ($this->workflows) { + $workflows = $this->helpWorkflows; + if ($workflows === null) { + $workflows = $this->workflows; + } + + if ($workflows) { $has_help = false; $out[] = $this->format('**%s**', pht('WORKFLOWS')); $out[] = null; - $flows = $this->workflows; + $flows = $workflows; ksort($flows); foreach ($flows as $workflow) { if ($workflow->getName() == 'help') { @@ -739,7 +757,12 @@ final class PhutilArgumentParser extends Phobject { $indent = ($show_details ? 0 : 6); - $workflow = idx($this->workflows, strtolower($workflow_name)); + $workflows = $this->helpWorkflows; + if ($workflows === null) { + $workflows = $this->workflows; + } + + $workflow = idx($workflows, strtolower($workflow_name)); if (!$workflow) { $out[] = $this->indent( $indent, diff --git a/src/parser/argument/workflow/PhutilHelpArgumentWorkflow.php b/src/parser/argument/workflow/PhutilHelpArgumentWorkflow.php index 7257f68f..d464e1cf 100644 --- a/src/parser/argument/workflow/PhutilHelpArgumentWorkflow.php +++ b/src/parser/argument/workflow/PhutilHelpArgumentWorkflow.php @@ -2,17 +2,6 @@ final class PhutilHelpArgumentWorkflow extends PhutilArgumentWorkflow { - private $runtime; - - public function setRuntime($runtime) { - $this->runtime = $runtime; - return $this; - } - - public function getRuntime() { - return $this->runtime; - } - protected function didConstruct() { $this->setName('help'); $this->setExamples(<<getArg('help-with-what'); - $runtime = $this->getRuntime(); - if ($runtime) { - $toolset = $runtime->getToolset(); - if ($toolset->getToolsetKey() === 'arc') { - $workflows = $args->getWorkflows(); - - $legacy = array(); - - $legacy[] = new ArcanistCloseRevisionWorkflow(); - $legacy[] = new ArcanistCommitWorkflow(); - $legacy[] = new ArcanistCoverWorkflow(); - $legacy[] = new ArcanistDiffWorkflow(); - $legacy[] = new ArcanistExportWorkflow(); - $legacy[] = new ArcanistGetConfigWorkflow(); - $legacy[] = new ArcanistSetConfigWorkflow(); - $legacy[] = new ArcanistInstallCertificateWorkflow(); - $legacy[] = new ArcanistLandWorkflow(); - $legacy[] = new ArcanistLintersWorkflow(); - $legacy[] = new ArcanistLintWorkflow(); - $legacy[] = new ArcanistListWorkflow(); - $legacy[] = new ArcanistPatchWorkflow(); - $legacy[] = new ArcanistPasteWorkflow(); - $legacy[] = new ArcanistTasksWorkflow(); - $legacy[] = new ArcanistTodoWorkflow(); - $legacy[] = new ArcanistUnitWorkflow(); - $legacy[] = new ArcanistWhichWorkflow(); - - foreach ($legacy as $workflow) { - // If this workflow has been updated but not removed from the list - // above yet, just skip it. - if ($workflow instanceof ArcanistArcWorkflow) { - continue; - } - - $workflows[] = $workflow->newLegacyPhutilWorkflow(); - } - - $args->setWorkflows($workflows); - } - } - if (!$with) { $args->printHelpAndExit(); } else { diff --git a/src/runtime/ArcanistRuntime.php b/src/runtime/ArcanistRuntime.php index bed9e5cc..07a6ce1b 100644 --- a/src/runtime/ArcanistRuntime.php +++ b/src/runtime/ArcanistRuntime.php @@ -162,6 +162,9 @@ final class ArcanistRuntime { // TOOLSETS: Some day, stop falling through to the old "arc" runtime. + $help_workflows = $this->getHelpWorkflows($phutil_workflows); + $args->setHelpWorkflows($help_workflows); + try { return $args->parseWorkflowsFull($phutil_workflows); } catch (ArcanistMissingArgumentTerminatorException $terminator_exception) { @@ -869,4 +872,41 @@ final class ArcanistRuntime { return $this->toolset; } + private function getHelpWorkflows(array $workflows) { + if ($this->getToolset()->getToolsetKey() === 'arc') { + $legacy = array(); + + $legacy[] = new ArcanistCloseRevisionWorkflow(); + $legacy[] = new ArcanistCommitWorkflow(); + $legacy[] = new ArcanistCoverWorkflow(); + $legacy[] = new ArcanistDiffWorkflow(); + $legacy[] = new ArcanistExportWorkflow(); + $legacy[] = new ArcanistGetConfigWorkflow(); + $legacy[] = new ArcanistSetConfigWorkflow(); + $legacy[] = new ArcanistInstallCertificateWorkflow(); + $legacy[] = new ArcanistLandWorkflow(); + $legacy[] = new ArcanistLintersWorkflow(); + $legacy[] = new ArcanistLintWorkflow(); + $legacy[] = new ArcanistListWorkflow(); + $legacy[] = new ArcanistPatchWorkflow(); + $legacy[] = new ArcanistPasteWorkflow(); + $legacy[] = new ArcanistTasksWorkflow(); + $legacy[] = new ArcanistTodoWorkflow(); + $legacy[] = new ArcanistUnitWorkflow(); + $legacy[] = new ArcanistWhichWorkflow(); + + foreach ($legacy as $workflow) { + // If this workflow has been updated but not removed from the list + // above yet, just skip it. + if ($workflow instanceof ArcanistArcWorkflow) { + continue; + } + + $workflows[] = $workflow->newLegacyPhutilWorkflow(); + } + } + + return $workflows; + } + } diff --git a/src/toolset/ArcanistWorkflowArgument.php b/src/toolset/ArcanistWorkflowArgument.php index 0c1ab602..9a936b33 100644 --- a/src/toolset/ArcanistWorkflowArgument.php +++ b/src/toolset/ArcanistWorkflowArgument.php @@ -41,6 +41,11 @@ final class ArcanistWorkflowArgument $spec['param'] = $parameter; } + $help = $this->getHelp(); + if ($help !== null) { + $spec['help'] = $help; + } + return $spec; } diff --git a/src/toolset/workflow/ArcanistHelpWorkflow.php b/src/toolset/workflow/ArcanistHelpWorkflow.php index 8cda9ab2..e7ac38fc 100644 --- a/src/toolset/workflow/ArcanistHelpWorkflow.php +++ b/src/toolset/workflow/ArcanistHelpWorkflow.php @@ -8,8 +8,7 @@ final class ArcanistHelpWorkflow } public function newPhutilWorkflow() { - return id(new PhutilHelpArgumentWorkflow()) - ->setRuntime($this->getRuntime()); + return new PhutilHelpArgumentWorkflow(); } public function supportsToolset(ArcanistToolset $toolset) {