From df2c1ba912d1c5e19912a536871beb9a150c0424 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 25 Sep 2018 09:07:16 -0700 Subject: [PATCH] [Wilds] Provide a skeleton for prompt behaviors Summary: Ref T13098. Ref T13198. Ref T12996. The major ideas here are: Workflows must define a list of all the prompts they can raise, so that these prompts can be enumerated with `arc prompts `. Prompts themselves should respond properly to ^C (abort immediately) and we should be able to make them nonblocking in the future (particularly, we'd like to be able to continue reading bytes from subprocess buffers while the prompt is shown on screen). This doesn't have a lot of fancy features yet (non-confirm prompts, default yes, prompts which don't abort on "N", etc) but those should be easy to add later. In the future, you'll be able to configure a default answer to prompts either in a config file or at runtime with `--config prompts=x.y.z=N` or similar. This removes the history/readline mode for prompts, where you could use the up arrow to cycle through older responses. I believe this was only really useful for "excuse" prompts and intend to remove those. Test Plan: Forced `arc shell-complete` to always prompt, then: - Got prompted, answered "y", "n", "N", "", "quack". Got sensible behavior in all cases. - Ran `echo | arc shell-complete`, got a TTY error. - Ran `arc prompts`, `arc prompts shell-complete`. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13098, T13198, T12996 Differential Revision: https://secure.phabricator.com/D19706 --- src/__phutil_library_map__.php | 4 + src/log/ArcanistLogEngine.php | 13 +- src/toolset/ArcanistPrompt.php | 152 ++++++++++++++++++ src/toolset/ArcanistWorkflow.php | 60 +++++++ .../workflow/ArcanistPromptsWorkflow.php | 80 +++++++++ .../ArcanistShellCompleteWorkflow.php | 18 ++- support/ArcanistRuntime.php | 7 + 7 files changed, 326 insertions(+), 8 deletions(-) create mode 100644 src/toolset/ArcanistPrompt.php create mode 100644 src/toolset/workflow/ArcanistPromptsWorkflow.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4d62cbd8..cb68a9a5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -385,6 +385,8 @@ phutil_register_library_map(array( 'ArcanistPregQuoteMisuseXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPregQuoteMisuseXHPASTLinterRule.php', 'ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase.php', 'ArcanistProjectConfigurationSource' => 'config/source/ArcanistProjectConfigurationSource.php', + 'ArcanistPrompt' => 'toolset/ArcanistPrompt.php', + 'ArcanistPromptsWorkflow' => 'toolset/workflow/ArcanistPromptsWorkflow.php', 'ArcanistPublicPropertyXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistPublicPropertyXHPASTLinterRule.php', 'ArcanistPublicPropertyXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistPublicPropertyXHPASTLinterRuleTestCase.php', 'ArcanistPuppetLintLinter' => 'lint/linter/ArcanistPuppetLintLinter.php', @@ -1491,6 +1493,8 @@ phutil_register_library_map(array( 'ArcanistPregQuoteMisuseXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPregQuoteMisuseXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistProjectConfigurationSource' => 'ArcanistWorkingCopyConfigurationSource', + 'ArcanistPrompt' => 'Phobject', + 'ArcanistPromptsWorkflow' => 'ArcanistWorkflow', 'ArcanistPublicPropertyXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistPublicPropertyXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistPuppetLintLinter' => 'ArcanistExternalLinter', diff --git a/src/log/ArcanistLogEngine.php b/src/log/ArcanistLogEngine.php index b882f1e8..63736720 100644 --- a/src/log/ArcanistLogEngine.php +++ b/src/log/ArcanistLogEngine.php @@ -18,12 +18,19 @@ final class ArcanistLogEngine return new ArcanistLogMessage(); } + private function writeBytes($bytes) { + fprintf(STDERR, '%s', $bytes); + return $this; + } + + public function writeNewline() { + return $this->writeBytes("\n"); + } + public function writeMessage(ArcanistLogMessage $message) { $color = $message->getColor(); - fprintf( - STDERR, - '%s', + $this->writeBytes( tsprintf( "** %s ** %s\n", $message->getLabel(), diff --git a/src/toolset/ArcanistPrompt.php b/src/toolset/ArcanistPrompt.php new file mode 100644 index 00000000..9844de77 --- /dev/null +++ b/src/toolset/ArcanistPrompt.php @@ -0,0 +1,152 @@ +key = $key; + return $this; + } + + public function getKey() { + return $this->key; + } + + public function setWorkflow(ArcanistWorkflow $workflow) { + $this->workflow = $workflow; + return $this; + } + + public function getWorkflow() { + return $this->workflow; + } + + public function setDescription($description) { + $this->description = $description; + return $this; + } + + public function getDescription() { + return $this->description; + } + + public function setQuery($query) { + $this->query = $query; + return $this; + } + + public function getQuery() { + return $this->query; + } + + public function execute() { + $workflow = $this->getWorkflow(); + if ($workflow) { + $workflow_ok = $workflow->hasPrompt($this->getKey()); + } else { + $workflow_ok = false; + } + + if (!$workflow_ok) { + throw new Exception( + pht( + 'Prompt ("%s") is executing, but it is not properly bound to the '. + 'invoking workflow. You may have called "newPrompt()" to execute a '. + 'prompt instead of "getPrompt()". Use "newPrompt()" when defining '. + 'prompts and "getPrompt()" when executing them.', + $this->getKey())); + } + + $query = $this->getQuery(); + if (!strlen($query)) { + throw new Exception( + pht( + 'Prompt ("%s") has no query text!', + $this->getKey())); + } + + $options = '[y/N]'; + $default = 'N'; + + try { + phutil_console_require_tty(); + } catch (PhutilConsoleStdinNotInteractiveException $ex) { + // TOOLSETS: Clean this up to provide more details to the user about how + // they can configure prompts to be answered. + + // Throw after echoing the prompt so the user has some idea what happened. + echo $query."\n"; + throw $ex; + } + + // NOTE: We're making stdin nonblocking so that we can respond to signals + // immediately. If we don't, and you ^C during a prompt, the program does + // not handle the signal until fgets() returns. + + $stdin = fopen('php://stdin', 'r'); + if (!$stdin) { + throw new Exception(pht('Failed to open stdin for reading.')); + } + + $ok = stream_set_blocking($stdin, false); + if (!$ok) { + throw new Exception(pht('Unable to set stdin nonblocking.')); + } + + echo "\n"; + + $result = null; + while (true) { + echo tsprintf( + '** %s ** %s %s ', + '>>>', + $query, + $options); + + while (true) { + $read = array($stdin); + $write = array(); + $except = array(); + + $ok = stream_select($read, $write, $except, 1); + if ($ok === false) { + throw new Exception(pht('stream_select() failed!')); + } + + $response = fgets($stdin); + if (!strlen($response)) { + continue; + } + + break; + } + + $response = trim($response); + if (!strlen($response)) { + $response = $default; + } + + if (phutil_utf8_strtolower($response) == 'y') { + $result = true; + break; + } + + if (phutil_utf8_strtolower($response) == 'n') { + $result = false; + break; + } + } + + if (!$result) { + throw new ArcanistUserAbortException(); + } + + } + +} + diff --git a/src/toolset/ArcanistWorkflow.php b/src/toolset/ArcanistWorkflow.php index 2d229268..75a34583 100644 --- a/src/toolset/ArcanistWorkflow.php +++ b/src/toolset/ArcanistWorkflow.php @@ -8,6 +8,7 @@ abstract class ArcanistWorkflow extends Phobject { private $configurationEngine; private $configurationSourceList; private $conduitEngine; + private $promptMap; /** * Return the command used to invoke this workflow from the command like, @@ -203,4 +204,63 @@ abstract class ArcanistWorkflow extends Phobject { throw new PhutilMethodNotImplementedException(); } + protected function newPrompts() { + return array(); + } + + protected function newPrompt($key) { + return id(new ArcanistPrompt()) + ->setWorkflow($this) + ->setKey($key); + } + + public function hasPrompt($key) { + $map = $this->getPromptMap(); + return isset($map[$key]); + } + + public function getPromptMap() { + if ($this->promptMap === null) { + $prompts = $this->newPrompts(); + assert_instances_of($prompts, 'ArcanistPrompt'); + + $map = array(); + foreach ($prompts as $prompt) { + $key = $prompt->getKey(); + + if (isset($map[$key])) { + throw new Exception( + pht( + 'Workflow ("%s") generates two prompts with the same '. + 'key ("%s"). Each prompt a workflow generates must have a '. + 'unique key.', + get_class($this), + $key)); + } + + $map[$key] = $prompt; + } + + $this->promptMap = $map; + } + + return $this->promptMap; + } + + protected function getPrompt($key) { + $map = $this->getPromptMap(); + + $prompt = idx($map, $key); + if (!$prompt) { + throw new Exception( + pht( + 'Workflow ("%s") is requesting a prompt ("%s") but it did not '. + 'generate any prompt with that name in "newPrompts()".', + get_class($this), + $key)); + } + + return clone $prompt; + } + } diff --git a/src/toolset/workflow/ArcanistPromptsWorkflow.php b/src/toolset/workflow/ArcanistPromptsWorkflow.php new file mode 100644 index 00000000..fa9d78fe --- /dev/null +++ b/src/toolset/workflow/ArcanistPromptsWorkflow.php @@ -0,0 +1,80 @@ + +EOTEXT +); + + return $this->newWorkflowInformation() + ->addExample(pht('**prompts** __workflow__')) + ->setHelp($help); + } + + public function getWorkflowArguments() { + return array( + $this->newWorkflowArgument('argv') + ->setWildcard(true), + ); + } + + public function runWorkflow() { + $argv = $this->getArgument('argv'); + + if (!$argv) { + throw new PhutilArgumentUsageException( + pht('Provide a workflow to list prompts for.')); + } + + $runtime = $this->getRuntime(); + $workflows = $runtime->getWorkflows(); + + $workflow_key = array_shift($argv); + $workflow = idx($workflows, $workflow_key); + + if (!$workflow) { + throw new PhutilArgumentUsageException( + pht( + 'Workflow "%s" is unknown. Supported workflows are: %s.', + $workflow_key, + implode(', ', array_keys($workflows)))); + } + + $prompts = $workflow->getPromptMap(); + if (!$prompts) { + echo tsprintf( + "%s\n", + pht('This workflow can not prompt.')); + return 0; + } + + foreach ($prompts as $prompt) { + echo tsprintf( + "**%s**\n", + $prompt->getKey()); + echo tsprintf( + "%s\n", + $prompt->getDescription()); + } + + return 0; + } + +} diff --git a/src/toolset/workflow/ArcanistShellCompleteWorkflow.php b/src/toolset/workflow/ArcanistShellCompleteWorkflow.php index b3e7b2fa..04400366 100644 --- a/src/toolset/workflow/ArcanistShellCompleteWorkflow.php +++ b/src/toolset/workflow/ArcanistShellCompleteWorkflow.php @@ -155,6 +155,16 @@ EOTEXT $this->runAutocomplete(); } + protected function newPrompts() { + return array( + $this->newPrompt('arc.shell-complete.install') + ->setDescription( + pht( + 'Confirms writing to to "~/.profile" (or another similar file) '. + 'to install shell completion.')), + ); + } + private function runInstall() { $log = $this->getLogEngine(); @@ -281,11 +291,9 @@ EOTEXT } } - // TOOLSETS: Generalize prompting. - - if (!phutil_console_confirm($prompt, false)) { - throw new PhutilArgumentUsageException(pht('Aborted.')); - } + $this->getPrompt('arc.shell-complete.install') + ->setQuery($prompt) + ->execute(); Filesystem::writeFile($file_path, $new_data); diff --git a/support/ArcanistRuntime.php b/support/ArcanistRuntime.php index 158db813..1914cb38 100644 --- a/support/ArcanistRuntime.php +++ b/support/ArcanistRuntime.php @@ -32,6 +32,8 @@ final class ArcanistRuntime { $log->writeError(pht('CONDUIT'), $ex->getMessage()); } catch (PhutilArgumentUsageException $ex) { $log->writeError(pht('USAGE EXCEPTION'), $ex->getMessage()); + } catch (ArcanistUserAbortException $ex) { + $log->writeError(pht('---'), $ex->getMessage()); } return 1; @@ -590,6 +592,11 @@ final class ArcanistRuntime { } } + // It's common for users to ^C on prompts. Write a newline before writing + // a response to the interrupt so the behavior is a little cleaner. This + // also avoids lines that read "^C [ INTERRUPT ] ...". + $log->writeNewline(); + if ($should_exit) { $log->writeHint( pht('INTERRUPT'),