From c49eeb235ea0276dc34e9f87404b696642495f5a Mon Sep 17 00:00:00 2001 From: Waldir Pimenta Date: Thu, 7 Dec 2023 16:21:39 +0000 Subject: [PATCH] Improve command line prompts in setup issue pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: This is a follow-up to D25425, where these improvements to the CLI prompt markers were discussed. Changes included in this revision: - Build all prompts the same way - Remove space after the prompt marker (add it via CSS instead) - Add server path prefix - Make the prompt unselectable Test Plan: - Visit any of the setup issue pages, e.g. /config/issue/auth.config-unlocked/ (after ensuring that the corresponding issue is present — in this case, by doing `./bin/auth unlock`) - For example, Deactivate all PHP extensions to trigger each /config/issue/extension.gd/ etc. - For example, update at least up to `dc10a7e69ea3` to see the database upgrade tip etc. - Confirm that the command line prompts now include the path prefix - Confirm that selecting the command via double-click (or click-and-drag) does not select the prompt Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Tags: #ux, #config Differential Revision: https://we.phorge.it/D25466 --- resources/celerity/map.php | 4 ++-- .../config/check/PhabricatorAuthSetupCheck.php | 3 ++- .../config/check/PhabricatorBaseURISetupCheck.php | 7 +++---- .../config/check/PhabricatorDaemonsSetupCheck.php | 10 ++++++++-- .../check/PhabricatorDatabaseSetupCheck.php | 15 +++++++++++---- .../check/PhabricatorElasticsearchSetupCheck.php | 10 ++++++++-- .../check/PhabricatorExtraConfigSetupCheck.php | 14 ++++++++++---- .../config/view/PhabricatorSetupIssueView.php | 9 +++++---- .../examples/PhabricatorSetupIssueUIExample.php | 8 ++++++-- .../rsrc/css/application/config/setup-issue.css | 5 +++++ 10 files changed, 60 insertions(+), 25 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index d0bb7523a7..12ad67b011 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -45,7 +45,7 @@ return array( 'rsrc/css/application/conduit/conduit-api.css' => 'ce2cfc41', 'rsrc/css/application/config/config-options.css' => '16c920ae', 'rsrc/css/application/config/config-template.css' => 'e689dbbd', - 'rsrc/css/application/config/setup-issue.css' => '5eed85b2', + 'rsrc/css/application/config/setup-issue.css' => '93231115', 'rsrc/css/application/config/unhandled-exception.css' => '9ecfc00d', 'rsrc/css/application/conpherence/color.css' => 'b17746b0', 'rsrc/css/application/conpherence/durable-column.css' => '2d57072b', @@ -903,7 +903,7 @@ return array( 'project-card-view-css' => 'c1200da7', 'project-triggers-css' => 'cd9c8bb9', 'project-view-css' => '2f7caa20', - 'setup-issue-css' => '5eed85b2', + 'setup-issue-css' => '93231115', 'sprite-login-css' => '07052ee0', 'sprite-tokens-css' => 'f1896dc5', 'syntax-default-css' => '055fc231', diff --git a/src/applications/config/check/PhabricatorAuthSetupCheck.php b/src/applications/config/check/PhabricatorAuthSetupCheck.php index b971404c38..8705047263 100644 --- a/src/applications/config/check/PhabricatorAuthSetupCheck.php +++ b/src/applications/config/check/PhabricatorAuthSetupCheck.php @@ -74,7 +74,8 @@ final class PhabricatorAuthSetupCheck extends PhabricatorSetupCheck { ->addRelatedPhabricatorConfig('auth.lock-config') ->addCommand( hsprintf( - '$ ./bin/auth lock')); + '%s $./bin/auth lock', + PlatformSymbols::getPlatformServerPath())); } } } diff --git a/src/applications/config/check/PhabricatorBaseURISetupCheck.php b/src/applications/config/check/PhabricatorBaseURISetupCheck.php index 92e46641d7..e73c1455bf 100644 --- a/src/applications/config/check/PhabricatorBaseURISetupCheck.php +++ b/src/applications/config/check/PhabricatorBaseURISetupCheck.php @@ -96,9 +96,8 @@ final class PhabricatorBaseURISetupCheck extends PhabricatorSetupCheck { ->setMessage($message) ->addCommand( hsprintf( - '$ %s', - csprintf( - './bin/config set phabricator.base-uri %s', - $base_uri_guess))); + '%s $./bin/config set phabricator.base-uri %s', + PlatformSymbols::getPlatformServerPath(), + $base_uri_guess)); } } diff --git a/src/applications/config/check/PhabricatorDaemonsSetupCheck.php b/src/applications/config/check/PhabricatorDaemonsSetupCheck.php index df5821665c..ceedd5137e 100644 --- a/src/applications/config/check/PhabricatorDaemonsSetupCheck.php +++ b/src/applications/config/check/PhabricatorDaemonsSetupCheck.php @@ -49,7 +49,10 @@ final class PhabricatorDaemonsSetupCheck extends PhabricatorSetupCheck { ->setName(pht('Daemons Are Not Running')) ->setSummary($summary) ->setMessage($message) - ->addCommand('$ ./bin/phd start'); + ->addCommand( + hsprintf( + '%s $./bin/phd start', + PlatformSymbols::getPlatformServerPath())); } $expect_user = PhabricatorEnv::getEnvConfig('phd.user'); @@ -90,7 +93,10 @@ final class PhabricatorDaemonsSetupCheck extends PhabricatorSetupCheck { ->setSummary($summary) ->setMessage($message) ->addPhabricatorConfig('phd.user') - ->addCommand('$ ./bin/phd restart'); + ->addCommand( + hsprintf( + '%s $./bin/phd restart', + PlatformSymbols::getPlatformServerPath())); break; } diff --git a/src/applications/config/check/PhabricatorDatabaseSetupCheck.php b/src/applications/config/check/PhabricatorDatabaseSetupCheck.php index d3f6d52f04..99815d85e6 100644 --- a/src/applications/config/check/PhabricatorDatabaseSetupCheck.php +++ b/src/applications/config/check/PhabricatorDatabaseSetupCheck.php @@ -35,11 +35,13 @@ final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck { ->addPhabricatorConfig('mysql.port') ->addCommand( hsprintf( - '$ ./bin/config set mysql.host %s', + '%s $./bin/config set mysql.host %s', + PlatformSymbols::getPlatformServerPath(), $host)) ->addCommand( hsprintf( - '$ ./bin/config set mysql.port %s', + '%s $./bin/config set mysql.port %s', + PlatformSymbols::getPlatformServerPath(), $port)); } @@ -134,7 +136,10 @@ final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck { ->setName(pht('Setup MySQL Schema')) ->setMessage($message) ->setIsFatal(true) - ->addCommand(hsprintf('$ ./bin/storage upgrade')); + ->addCommand( + hsprintf( + '%s $./bin/storage upgrade', + PlatformSymbols::getPlatformServerPath())); return true; } @@ -160,7 +165,9 @@ final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck { ->setIsFatal(true) ->setMessage($message) ->addCommand( - hsprintf('$ ./bin/storage upgrade')); + hsprintf( + '%s $./bin/storage upgrade', + PlatformSymbols::getPlatformServerPath())); return true; } diff --git a/src/applications/config/check/PhabricatorElasticsearchSetupCheck.php b/src/applications/config/check/PhabricatorElasticsearchSetupCheck.php index 8466c5a6c6..268f55f80b 100644 --- a/src/applications/config/check/PhabricatorElasticsearchSetupCheck.php +++ b/src/applications/config/check/PhabricatorElasticsearchSetupCheck.php @@ -60,7 +60,10 @@ final class PhabricatorElasticsearchSetupCheck extends PhabricatorSetupCheck { $this ->newIssue('elastic.missing-index') ->setName(pht('Elasticsearch Index Not Found')) - ->addCommand('./bin/search init') + ->addCommand( + hsprintf( + '%s $./bin/search init', + PlatformSymbols::getPlatformServerPath())) ->setSummary($summary) ->setMessage($message); @@ -76,7 +79,10 @@ final class PhabricatorElasticsearchSetupCheck extends PhabricatorSetupCheck { $this ->newIssue('elastic.broken-index') ->setName(pht('Elasticsearch Index Schema Mismatch')) - ->addCommand('./bin/search init') + ->addCommand( + hsprintf( + '%s $./bin/search init', + PlatformSymbols::getPlatformServerPath())) ->setSummary($summary) ->setMessage($message); } diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index ae14a0ab0a..070127ccbd 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -76,7 +76,10 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { $issue->setMessage($message); if ($found_local) { - $command = csprintf('$ ./bin/config delete %s', $key); + $command = hsprintf( + '%s $./bin/config delete %s', + PlatformSymbols::getPlatformServerPath(), + $key); $issue->addCommand($command); } @@ -166,9 +169,12 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'target' => '_blank', ), $doc_name)); - $command = csprintf( - '$ ./bin/config delete --database %R', - $key); + $command = hsprintf( + '%s $%s', + PlatformSymbols::getPlatformServerPath(), + csprintf( + './bin/config delete --database %R', + $key)); $this->newIssue('config.locked.'.$key) ->setShortName(pht('Deprecated Config Source')) diff --git a/src/applications/config/view/PhabricatorSetupIssueView.php b/src/applications/config/view/PhabricatorSetupIssueView.php index e07e33b3d0..9aff749bb2 100644 --- a/src/applications/config/view/PhabricatorSetupIssueView.php +++ b/src/applications/config/view/PhabricatorSetupIssueView.php @@ -83,11 +83,11 @@ final class PhabricatorSetupIssueView extends AphrontView { // TODO: We should do a better job of detecting how to install extensions // on the current system. $install_commands = hsprintf( - "\$ sudo apt-get install php-extname ". + "$sudo apt-get install php-extname ". "# Debian / Ubuntu\n". - "\$ sudo dnf install php-extname ". + "$sudo dnf install php-extname ". "# Red Hat / Derivatives\n". - "\$ sudo yum install php-extname ". + "$sudo yum install php-extname ". "# Older Red Hat versions"); $fallback_info = pht( @@ -286,7 +286,8 @@ final class PhabricatorSetupIssueView extends AphrontView { $update = array(); foreach ($configs as $key) { $update[] = hsprintf( - '$ ./bin/config set %s value', + '%s $./bin/config set %s value', + PlatformSymbols::getPlatformServerPath(), $key); } $update = phutil_tag('pre', array(), phutil_implode_html("\n", $update)); diff --git a/src/applications/uiexample/examples/PhabricatorSetupIssueUIExample.php b/src/applications/uiexample/examples/PhabricatorSetupIssueUIExample.php index d6386e59d4..4343bcb8cb 100644 --- a/src/applications/uiexample/examples/PhabricatorSetupIssueUIExample.php +++ b/src/applications/uiexample/examples/PhabricatorSetupIssueUIExample.php @@ -24,8 +24,12 @@ final class PhabricatorSetupIssueUIExample extends PhabricatorUIExample { ->setSummary(pht('Summary')) ->setMessage(pht('Message')) ->setIssueKey('example.key') - ->addCommand('$ # Add Command') - ->addCommand(hsprintf('$ %s', '$ ls -1 > /dev/null')) + ->addCommand(hsprintf( + '%s $# Add Command', + PlatformSymbols::getPlatformServerPath())) + ->addCommand(hsprintf( + '%s $ls -1 > /dev/null', + PlatformSymbols::getPlatformServerPath())) ->addPHPConfig('php.config.example') ->addPhabricatorConfig('test.value') ->addPHPExtension('libexample'); diff --git a/webroot/rsrc/css/application/config/setup-issue.css b/webroot/rsrc/css/application/config/setup-issue.css index a80b02815c..4af5e9f417 100644 --- a/webroot/rsrc/css/application/config/setup-issue.css +++ b/webroot/rsrc/css/application/config/setup-issue.css @@ -122,6 +122,11 @@ padding: 12px 0; } +.setup-issue-config > pre > tt { + user-select: none; + margin-right: 0.5em; +} + .setup-issue-config + .setup-issue-config { padding-top: 0; }