From c79094d7a8bdf053e9b58d80f8375d6e148fc24b Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 18 Apr 2020 15:59:30 -0700 Subject: [PATCH] Add static errors, supported protocols, and a dynamic function listing to external editor settings page Summary: Ref T13515. - Previously valid editor URIs may become invalid without being changed (if an administrator removes a protocol from the list, for example), but this isn't explained very well. Show an error on the settings page if the current value isn't usable. - Generate a list of functions from an authority in the parser. - Generate a list of protocols from configuration. Test Plan: {F7370872} Maniphest Tasks: T13515 Differential Revision: https://secure.phabricator.com/D21146 --- .../editor/PhabricatorSettingsEditEngine.php | 30 +++- .../PhabricatorEditEngineSettingsPanel.php | 11 ++ ...PhabricatorExternalEditorSettingsPanel.php | 146 ++++++++++++++++++ .../setting/PhabricatorEditorSetting.php | 23 +-- .../user/userguide/external_editor.diviner | 24 +-- .../editor/PhabricatorEditorURIEngine.php | 4 + 6 files changed, 214 insertions(+), 24 deletions(-) diff --git a/src/applications/settings/editor/PhabricatorSettingsEditEngine.php b/src/applications/settings/editor/PhabricatorSettingsEditEngine.php index 218b5b82b0..b45de2dce1 100644 --- a/src/applications/settings/editor/PhabricatorSettingsEditEngine.php +++ b/src/applications/settings/editor/PhabricatorSettingsEditEngine.php @@ -7,6 +7,7 @@ final class PhabricatorSettingsEditEngine private $isSelfEdit; private $profileURI; + private $settingsPanel; public function setIsSelfEdit($is_self_edit) { $this->isSelfEdit = $is_self_edit; @@ -26,6 +27,15 @@ final class PhabricatorSettingsEditEngine return $this->profileURI; } + public function setSettingsPanel($settings_panel) { + $this->settingsPanel = $settings_panel; + return $this; + } + + public function getSettingsPanel() { + return $this->settingsPanel; + } + public function isEngineConfigurable() { return false; } @@ -252,13 +262,29 @@ final class PhabricatorSettingsEditEngine protected function newEditFormHeadContent( PhabricatorEditEnginePageState $state) { + $content = array(); + if ($state->getIsSave()) { - return id(new PHUIInfoView()) + $content[] = id(new PHUIInfoView()) ->setSeverity(PHUIInfoView::SEVERITY_NOTICE) ->appendChild(pht('Changes saved.')); } - return null; + $panel = $this->getSettingsPanel(); + $content[] = $panel->newSettingsPanelEditFormHeadContent($state); + + return $content; + } + + protected function newEditFormTailContent( + PhabricatorEditEnginePageState $state) { + + $content = array(); + + $panel = $this->getSettingsPanel(); + $content[] = $panel->newSettingsPanelEditFormTailContent($state); + + return $content; } } diff --git a/src/applications/settings/panel/PhabricatorEditEngineSettingsPanel.php b/src/applications/settings/panel/PhabricatorEditEngineSettingsPanel.php index 5aad71785e..a4c9c3b789 100644 --- a/src/applications/settings/panel/PhabricatorEditEngineSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorEditEngineSettingsPanel.php @@ -22,6 +22,7 @@ abstract class PhabricatorEditEngineSettingsPanel $engine = id(new PhabricatorSettingsEditEngine()) ->setController($this->getController()) ->setNavigation($this->getNavigation()) + ->setSettingsPanel($this) ->setIsSelfEdit($is_self) ->setProfileURI($profile_uri); @@ -71,4 +72,14 @@ abstract class PhabricatorEditEngineSettingsPanel return mpull($panel_settings, 'getSettingKey'); } + public function newSettingsPanelEditFormHeadContent( + PhabricatorEditEnginePageState $state) { + return null; + } + + public function newSettingsPanelEditFormTailContent( + PhabricatorEditEnginePageState $state) { + return null; + } + } diff --git a/src/applications/settings/panel/PhabricatorExternalEditorSettingsPanel.php b/src/applications/settings/panel/PhabricatorExternalEditorSettingsPanel.php index 9e402b1492..d9e8736954 100644 --- a/src/applications/settings/panel/PhabricatorExternalEditorSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorExternalEditorSettingsPanel.php @@ -21,4 +21,150 @@ final class PhabricatorExternalEditorSettingsPanel return true; } + public function newSettingsPanelEditFormHeadContent( + PhabricatorEditEnginePageState $state) { + + // The "Editor" setting stored in the database may be invalidated by + // configuration or software changes. If a saved URI becomes invalid + // (for example, its protocol is removed from the list of allowed + // protocols), it will stop working. + + // If the stored value has a problem like this, show a static error + // message without requiring the user to save changes. + + if ($state->getIsSubmit()) { + return null; + } + + $viewer = $this->getViewer(); + $pattern = $viewer->getUserSetting(PhabricatorEditorSetting::SETTINGKEY); + + if (!strlen($pattern)) { + return null; + } + + $caught = null; + try { + id(new PhabricatorEditorURIEngine()) + ->setPattern($pattern) + ->validatePattern(); + } catch (PhabricatorEditorURIParserException $ex) { + $caught = $ex; + } + + if (!$caught) { + return null; + } + + return id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->appendChild($caught->getMessage()); + } + + public function newSettingsPanelEditFormTailContent( + PhabricatorEditEnginePageState $state) { + $viewer = $this->getViewer(); + + $variables = PhabricatorEditorURIEngine::getVariableDefinitions(); + + $rows = array(); + foreach ($variables as $key => $variable) { + $rows[] = array( + phutil_tag('tt', array(), '%'.$key), + $variable['name'], + $variable['example'], + ); + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Variable'), + pht('Replaced With'), + pht('Example'), + )) + ->setColumnClasses( + array( + 'center', + 'pri', + 'wide', + )); + + $variables_box = id(new PHUIObjectBoxView()) + ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) + ->setHeaderText(pht('External Editor URI Variables')) + ->setTable($table); + + $label_map = array( + 'http' => pht('Hypertext Transfer Protocol'), + 'https' => pht('Hypertext Transfer Protocol over SSL'), + 'txmt' => pht('TextMate'), + 'mvim' => pht('MacVim'), + 'subl' => pht('Sublime Text'), + 'vim' => pht('Vim'), + 'emacs' => pht('Emacs'), + 'vscode' => pht('Visual Studio Code'), + 'editor' => pht('Generic Editor'), + ); + + $default_label = phutil_tag('em', array(), pht('Supported Protocol')); + + $config_key = 'uri.allowed-editor-protocols'; + + $protocols = PhabricatorEnv::getEnvConfig($config_key); + $protocols = array_keys($protocols); + sort($protocols); + + $protocol_rows = array(); + foreach ($protocols as $protocol) { + $label = idx($label_map, $protocol, $default_label); + + $protocol_rows[] = array( + $protocol.'://', + $label, + ); + } + + $protocol_table = id(new AphrontTableView($protocol_rows)) + ->setNoDataString( + pht( + 'Phabricator is not configured to allow any editor protocols.')) + ->setHeaders( + array( + pht('Protocol'), + pht('Description'), + )) + ->setColumnClasses( + array( + 'pri', + 'wide', + )); + + $is_admin = $viewer->getIsAdmin(); + + $configure_button = id(new PHUIButtonView()) + ->setTag('a') + ->setText(pht('View Configuration')) + ->setIcon('fa-sliders') + ->setHref( + urisprintf( + '/config/edit/%s/', + $config_key)) + ->setDisabled(!$is_admin); + + $protocol_header = id(new PHUIHeaderView()) + ->setHeader(pht('Supported Editor Protocols')) + ->addActionLink($configure_button); + + $protocols_box = id(new PHUIObjectBoxView()) + ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) + ->setHeader($protocol_header) + ->setTable($protocol_table); + + return array( + $variables_box, + $protocols_box, + ); + } + } diff --git a/src/applications/settings/setting/PhabricatorEditorSetting.php b/src/applications/settings/setting/PhabricatorEditorSetting.php index fd1fae8e70..6884217a3e 100644 --- a/src/applications/settings/setting/PhabricatorEditorSetting.php +++ b/src/applications/settings/setting/PhabricatorEditorSetting.php @@ -20,20 +20,23 @@ final class PhabricatorEditorSetting protected function getControlInstructions() { return pht( "Many text editors can be configured as URI handlers for special ". - "protocols like `editor://`. If you have such an editor, Phabricator ". - "can generate links that you can click to open files locally.". + "protocols like `editor://`. If you have installed and configured ". + "such an editor, Phabricator can generate links that you can click ". + "to open files locally.". "\n\n". - "These special variables are supported:". + "Provide a URI pattern for building external editor URIs in your ". + "environment. For example, if you use TextMate on macOS, the pattern ". + "for your machine look like this:". "\n\n". - "| Value | Replaced With |\n". - "|-------|---------------|\n". - "| `%%f` | Filename |\n". - "| `%%l` | Line Number |\n". - "| `%%r` | Repository Callsign |\n". - "| `%%%%` | Literal `%%` |\n". + "```name=\"Example: TextMate on macOS\"\n". + "%s\n". + "```\n". "\n\n". "For complete instructions on editor configuration, ". - "see **[[ %s | %s ]]**.", + "see **[[ %s | %s ]]**.". + "\n\n". + "See the tables below for a list of supported variables and protocols.", + 'txmt://open/?url=file:///Users/alincoln/editor_links/%r/%f&line=%l', PhabricatorEnv::getDoclink('User Guide: Configuring an External Editor'), pht('User Guide: Configuring an External Editor')); } diff --git a/src/docs/user/userguide/external_editor.diviner b/src/docs/user/userguide/external_editor.diviner index c6f55183c8..07aa9db290 100644 --- a/src/docs/user/userguide/external_editor.diviner +++ b/src/docs/user/userguide/external_editor.diviner @@ -3,12 +3,14 @@ Setting up an external editor to integrate with Diffusion and Differential. -= Overview = +Overview +======== You can configure a URI handler to allow you to open files from Differential and Diffusion in your preferred text editor. -= Configuring Editors = +Configuring Editors +=================== To configure an external editor, go to {nav Settings > Application Settings > External Editor} and set "Editor Link" to a URI pattern (see below). This @@ -17,15 +19,12 @@ Diffusion. In general, you'll set this field to something like: - lang=uri - editor://open/?file=%f +```lang=uri +editor://open/?file=%f +``` -Some editors support opening multiple files at once when filenames are separated -by spaces. If your editor supports this feature, set "Edit Multiple Files" to -"Supported". Otherwise, you can set it to "Not Supported" to disable "Open All" -buttons in the interface. - -== Configuring: TextMate on OS X == +Configuring: TextMate on macOS +============================== TextMate installs a `txmt://` handler by default, so it's easy to configure this feature if you use TextMate. @@ -40,5 +39,6 @@ example, if you're developing Phabricator, it might look like this: Then set your "Editor Link" to: - lang=uri - txmt://open/?url=file:///Users/alincoln/editor_links/%r/%f&line=%l +```lang=uri +txmt://open/?url=file:///Users/alincoln/editor_links/%r/%f&line=%l +``` diff --git a/src/infrastructure/editor/PhabricatorEditorURIEngine.php b/src/infrastructure/editor/PhabricatorEditorURIEngine.php index 43bfc23a5b..6abfa98677 100644 --- a/src/infrastructure/editor/PhabricatorEditorURIEngine.php +++ b/src/infrastructure/editor/PhabricatorEditorURIEngine.php @@ -92,15 +92,19 @@ final class PhabricatorEditorURIEngine return array( '%' => array( 'name' => pht('Literal Percent Symbol'), + 'example' => '%', ), 'r' => array( 'name' => pht('Repository Callsign'), + 'example' => 'XYZ', ), 'f' => array( 'name' => pht('File Name'), + 'example' => pht('path/to/source.c'), ), 'l' => array( 'name' => pht('Line Number'), + 'example' => '777', ), ); }