From 067c7f8a74d14120e8fd13c643b5fbf46a777f9c Mon Sep 17 00:00:00 2001 From: vrana Date: Mon, 16 Jan 2012 11:08:54 -0800 Subject: [PATCH] Display links to editor in Differential and Diffusion Summary: It is possible to open a file in editor by registering a custom URI scheme (pseudo-protocol). Some editors register it by default. Having links to open the file in external editor is productivity booster although it is a little bit harder to set up. There are several other tools using file_link_format configuration directive (XDebug, Symfony) to bind to this protocol. I've added the example with editor: protocol which can be used as a proxy to actual editor (used by Nette Framework: http://wiki.nette.org/en/howto-editor-link). Test Plan: Configure Editor Link in User Preferences. Register URI scheme in OS. Open a file in Diffusion. Click on the Edit button. Open a revision in Differential. Click on the Edit button. Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley Differential Revision: https://secure.phabricator.com/D1422 --- src/__celerity_resource_map__.php | 47 ++++++++++--------- .../DifferentialDiffViewController.php | 5 +- .../DifferentialRevisionViewController.php | 1 + .../DifferentialChangesetListView.php | 20 +++++++- .../change/DiffusionChangeController.php | 3 +- .../commit/DiffusionCommitController.php | 1 + .../file/DiffusionBrowseFileController.php | 21 ++++++++- ...rUserPreferenceSettingsPanelController.php | 11 +++++ .../PhabricatorUserPreferences.php | 1 + .../people/storage/user/PhabricatorUser.php | 15 ++++++ .../people/storage/user/__init__.php | 1 + .../differential/behavior-dropdown-menus.js | 9 ++++ 12 files changed, 107 insertions(+), 28 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 181012c0dc..83f42b1e8f 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -330,16 +330,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/javelin/lib/behavior.js', ), - 0 => - array( - 'uri' => '/res/14c48a9f/rsrc/js/javelin/lib/__tests__/behavior.js', - 'type' => 'js', - 'requires' => - array( - 0 => 'javelin-behavior', - ), - 'disk' => '/rsrc/js/javelin/lib/__tests__/behavior.js', - ), 'javelin-behavior-aphront-basic-tokenizer' => array( 'uri' => '/res/9be30797/rsrc/js/application/core/behavior-tokenizer.js', @@ -471,7 +461,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-dropdown-menus' => array( - 'uri' => '/res/acba60ad/rsrc/js/application/differential/behavior-dropdown-menus.js', + 'uri' => '/res/7bfb2fdb/rsrc/js/application/differential/behavior-dropdown-menus.js', 'type' => 'js', 'requires' => array( @@ -1437,18 +1427,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/css/application/objectselector/object-selector.css', ), - 'phabricator-prefab' => - array( - 'uri' => '/res/5784a112/rsrc/js/application/core/Prefab.js', - 'type' => 'js', - 'requires' => - array( - 0 => 'javelin-install', - 1 => 'javelin-util', - 2 => 'javelin-dom', - ), - 'disk' => '/rsrc/js/application/core/Prefab.js', - ), 'phabricator-profile-css' => array( 'uri' => '/res/9869d10b/rsrc/css/application/profile/profile-view.css', @@ -1467,6 +1445,29 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/css/application/profile/profile-header-view.css', ), + 0 => + array( + 'uri' => '/res/b6096fdd/rsrc/js/javelin/lib/__tests__/URI.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-uri', + 1 => 'javelin-php-serializer', + ), + 'disk' => '/rsrc/js/javelin/lib/__tests__/URI.js', + ), + 'phabricator-prefab' => + array( + 'uri' => '/res/5784a112/rsrc/js/application/core/Prefab.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-install', + 1 => 'javelin-util', + 2 => 'javelin-dom', + ), + 'disk' => '/rsrc/js/application/core/Prefab.js', + ), 'phabricator-remarkup-css' => array( 'uri' => '/res/39f358b8/rsrc/css/core/remarkup.css', diff --git a/src/applications/differential/controller/diffview/DifferentialDiffViewController.php b/src/applications/differential/controller/diffview/DifferentialDiffViewController.php index 6a81b7d756..bcc78e6801 100644 --- a/src/applications/differential/controller/diffview/DifferentialDiffViewController.php +++ b/src/applications/differential/controller/diffview/DifferentialDiffViewController.php @@ -1,7 +1,7 @@ setChangesets($changesets) - ->setRenderingReferences($refs); + ->setRenderingReferences($refs) + ->setUser($request->getUser()); return $this->buildStandardPageResponse( id(new DifferentialPrimaryPaneView()) diff --git a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php index 719c04e22c..d0338c5b45 100644 --- a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php @@ -238,6 +238,7 @@ class DifferentialRevisionViewController extends DifferentialController { $changeset_view->setChangesets($visible_changesets); $changeset_view->setEditable(!$viewer_is_anonymous); $changeset_view->setStandaloneViews(true); + $changeset_view->setUser($user); $changeset_view->setRevision($revision); $changeset_view->setDiff($target); $changeset_view->setRenderingReferences($rendering_references); diff --git a/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php b/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php index bb7e717327..020703f8a4 100644 --- a/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php +++ b/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php @@ -25,6 +25,7 @@ class DifferentialChangesetListView extends AphrontView { private $renderURI = '/differential/changeset/'; private $whitespace; private $standaloneViews; + private $user; private $symbolIndexes = array(); private $repository; private $diff; @@ -44,6 +45,11 @@ class DifferentialChangesetListView extends AphrontView { return $this; } + public function setUser(PhabricatorUser $user) { + $this->user = $user; + return $this; + } + public function setRevision(DifferentialRevision $revision) { $this->revision = $revision; return $this; @@ -133,6 +139,19 @@ class DifferentialChangesetListView extends AphrontView { $meta['rightURI'] = (string)$detail_uri->alter('view', 'new'); } + if ($this->user) { + $path = ltrim( + $changeset->getAbsoluteRepositoryPath($this->diff, $repository), + '/'); + $line = 1; // TODO: get first changed line + $editor_link = $this->user->loadEditorLink($path, $line, $repository); + if ($editor_link) { + $meta['editor'] = $editor_link; + } else { + $meta['editorConfigure'] = '/settings/page/preferences/'; + } + } + $detail_button = javelin_render_tag( 'a', array( @@ -145,7 +164,6 @@ class DifferentialChangesetListView extends AphrontView { "View Options \xE2\x96\xBC"); } - $detail->setChangeset($changeset); $detail->addButton($detail_button); $detail->setSymbolIndex(idx($this->symbolIndexes, $key)); diff --git a/src/applications/diffusion/controller/change/DiffusionChangeController.php b/src/applications/diffusion/controller/change/DiffusionChangeController.php index 80f2d754a9..79b01e8183 100644 --- a/src/applications/diffusion/controller/change/DiffusionChangeController.php +++ b/src/applications/diffusion/controller/change/DiffusionChangeController.php @@ -1,7 +1,7 @@ getRepository()->getCallsign().'/diff/'); $changeset_view->setWhitespace( DifferentialChangesetParser::WHITESPACE_SHOW_ALL); + $changeset_view->setUser($this->getRequest()->getUser()); $content[] = $this->buildCrumbs( array( diff --git a/src/applications/diffusion/controller/commit/DiffusionCommitController.php b/src/applications/diffusion/controller/commit/DiffusionCommitController.php index cdb6abe232..97696eb5c9 100644 --- a/src/applications/diffusion/controller/commit/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/commit/DiffusionCommitController.php @@ -200,6 +200,7 @@ class DiffusionCommitController extends DiffusionController { $change_list->setChangesets($changesets); $change_list->setRenderingReferences($references); $change_list->setRenderURI('/diffusion/'.$callsign.'/diff/'); + $change_list->setUser($user); // TODO: This is pretty awkward, unify the CSS between Diffusion and // Differential better. diff --git a/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php index 859420fa64..3b3fa1a1ca 100644 --- a/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php @@ -1,7 +1,7 @@ View'); $view_select_panel->appendChild($view_select_form); + + $user = $request->getUser(); + if ($user) { + $line = 1; + $repository = $this->getDiffusionRequest()->getRepository(); + $editor_link = $user->loadEditorLink($path, $line, $repository); + if ($editor_link) { + $view_select_panel->addButton( + phutil_render_tag( + 'a', + array( + 'href' => $editor_link, + 'class' => 'button', + ), + 'Edit' + )); + } + } + $view_select_panel->appendChild('
'); // Build the content of the file. diff --git a/src/applications/people/controller/settings/panels/preferences/PhabricatorUserPreferenceSettingsPanelController.php b/src/applications/people/controller/settings/panels/preferences/PhabricatorUserPreferenceSettingsPanelController.php index 189351c6eb..9a5e390972 100644 --- a/src/applications/people/controller/settings/panels/preferences/PhabricatorUserPreferenceSettingsPanelController.php +++ b/src/applications/people/controller/settings/panels/preferences/PhabricatorUserPreferenceSettingsPanelController.php @@ -26,6 +26,7 @@ class PhabricatorUserPreferenceSettingsPanelController $preferences = $user->loadPreferences(); $pref_monospaced = PhabricatorUserPreferences::PREFERENCE_MONOSPACED; + $pref_editor = PhabricatorUserPreferences::PREFERENCE_EDITOR; $pref_titles = PhabricatorUserPreferences::PREFERENCE_TITLES; if ($request->isFormPost()) { @@ -35,6 +36,7 @@ class PhabricatorUserPreferenceSettingsPanelController $monospaced = preg_replace('/[^a-z0-9 ,"]+/i', '', $monospaced); $preferences->setPreference($pref_titles, $request->getStr($pref_titles)); + $preferences->setPreference($pref_editor, $request->getStr($pref_editor)); $preferences->setPreference($pref_monospaced, $monospaced); $preferences->save(); @@ -64,6 +66,15 @@ EXAMPLE; 'text' => 'In page titles, show Tool names as plain text: [Differential]', ))) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel('Editor Link') + ->setName($pref_editor) + ->setCaption( + 'Link to edit files in external editor. '. + '%f is replaced by filename, %l by line number, %r by repository. '. + 'Example: editor://open/?file=%f&line=%l&repository=%r') + ->setValue($preferences->getPreference($pref_editor))) ->appendChild( id(new AphrontFormTextControl()) ->setLabel('Monospaced Font') diff --git a/src/applications/people/storage/preferences/PhabricatorUserPreferences.php b/src/applications/people/storage/preferences/PhabricatorUserPreferences.php index 06c47de689..193aca39e4 100644 --- a/src/applications/people/storage/preferences/PhabricatorUserPreferences.php +++ b/src/applications/people/storage/preferences/PhabricatorUserPreferences.php @@ -19,6 +19,7 @@ class PhabricatorUserPreferences extends PhabricatorUserDAO { const PREFERENCE_MONOSPACED = 'monospaced'; + const PREFERENCE_EDITOR = 'editor'; const PREFERENCE_TITLES = 'titles'; const PREFERENCE_RE_PREFIX = 're-prefix'; diff --git a/src/applications/people/storage/user/PhabricatorUser.php b/src/applications/people/storage/user/PhabricatorUser.php index 12a8429856..0d932d705f 100644 --- a/src/applications/people/storage/user/PhabricatorUser.php +++ b/src/applications/people/storage/user/PhabricatorUser.php @@ -403,6 +403,7 @@ class PhabricatorUser extends PhabricatorUserDAO { $default_dict = array( PhabricatorUserPreferences::PREFERENCE_TITLES => 'glyph', + PhabricatorUserPreferences::PREFERENCE_EDITOR => '', PhabricatorUserPreferences::PREFERENCE_MONOSPACED => ''); $preferences->setPreferences($default_dict); @@ -412,6 +413,20 @@ class PhabricatorUser extends PhabricatorUserDAO { return $preferences; } + public function loadEditorLink($path, + $line, + PhabricatorRepository $repository) { + $editor = $this->loadPreferences()->getPreference( + PhabricatorUserPreferences::PREFERENCE_EDITOR); + if ($editor) { + return strtr($editor, array( + '%f' => phutil_escape_uri($path), + '%l' => phutil_escape_uri($line), + '%r' => phutil_escape_uri($repository->getCallsign()), + )); + } + } + private static function tokenizeName($name) { if (function_exists('mb_strtolower')) { $name = mb_strtolower($name, 'UTF-8'); diff --git a/src/applications/people/storage/user/__init__.php b/src/applications/people/storage/user/__init__.php index 29e3e21402..910e09c634 100644 --- a/src/applications/people/storage/user/__init__.php +++ b/src/applications/people/storage/user/__init__.php @@ -20,6 +20,7 @@ phutil_require_module('phabricator', 'storage/qsprintf'); phutil_require_module('phabricator', 'storage/queryfx'); phutil_require_module('phutil', 'filesystem'); +phutil_require_module('phutil', 'markup'); phutil_require_module('phutil', 'parser/uri'); phutil_require_module('phutil', 'utils'); diff --git a/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js b/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js index 3afe7b4598..73dba5650b 100644 --- a/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js +++ b/webroot/rsrc/js/application/differential/behavior-dropdown-menus.js @@ -56,6 +56,15 @@ JX.behavior('differential-dropdown-menus', function(config) { if (data.rightURI) { menu.addItem(link_to('Show Raw File (Right)', data.rightURI)); } + if (data.editor) { + menu.addItem(new JX.PhabricatorMenuItem( + 'Open in Editor', + JX.bind(null, location.assign, data.editor), // Open in the same window. + data.editor)); + } + if (data.editorConfigure) { + menu.addItem(link_to('Configure Editor', data.editorConfigure)); + } menu.listen( 'open',