Add an "Open in External Editor" keystroke to Differential

Summary: Ref T13515. See PHI1661. If a file is selected, add a keystroke to click the "Open in External Editor" link.

Test Plan: In Safari, Chrome, and Firefox: used "J" to select a file, then "\" to open it in an external editor. (In Safari and Chrome, this prompts.)

Maniphest Tasks: T13515

Differential Revision: https://secure.phabricator.com/D21135

resources/celerity/map.php

@@ -12,7 +12,7 @@ return array(
     'core.pkg.css' => '86f155f9',
     'core.pkg.js' => '705aec2c',
     'differential.pkg.css' => '607c84be',
-    'differential.pkg.js' => '1b97518d',
+    'differential.pkg.js' => 'd73a942b',
     'diffusion.pkg.css' => '42c75c37',
     'diffusion.pkg.js' => 'a98c0bf7',
     'maniphest.pkg.css' => '35995d6d',
@@ -377,8 +377,8 @@ return array(
     'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be',
     'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9',
     'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
-    'rsrc/js/application/diff/DiffChangeset.js' => 'a31ffc00',
+    'rsrc/js/application/diff/DiffChangeset.js' => '7ccc4153',
-    'rsrc/js/application/diff/DiffChangesetList.js' => '0f5c016d',
+    'rsrc/js/application/diff/DiffChangesetList.js' => '2e636e0a',
     'rsrc/js/application/diff/DiffInline.js' => 'a4a14a94',
     'rsrc/js/application/diff/behavior-preview-link.js' => 'f51e9c17',
     'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
@@ -774,8 +774,8 @@ return array(
     'phabricator-darklog' => '3b869402',
     'phabricator-darkmessage' => '26cd4b73',
     'phabricator-dashboard-css' => '5a205b9d',
-    'phabricator-diff-changeset' => 'a31ffc00',
+    'phabricator-diff-changeset' => '7ccc4153',
-    'phabricator-diff-changeset-list' => '0f5c016d',
+    'phabricator-diff-changeset-list' => '2e636e0a',
     'phabricator-diff-inline' => 'a4a14a94',
     'phabricator-drag-and-drop-file-upload' => '4370900d',
     'phabricator-draggable-list' => '0169e425',
@@ -1015,10 +1015,6 @@ return array(
       'javelin-workflow',
       'phuix-icon-view',
     ),
-    '0f5c016d' => array(
-      'javelin-install',
-      'phuix-button-view',
-    ),
     '111bfd2d' => array(
       'javelin-install',
     ),
@@ -1173,6 +1169,10 @@ return array(
       'javelin-util',
       'javelin-stratcom',
     ),
+    '2e636e0a' => array(
+      'javelin-install',
+      'phuix-button-view',
+    ),
     '2f1db1ed' => array(
       'javelin-util',
     ),
@@ -1612,6 +1612,17 @@ return array(
       'javelin-install',
       'javelin-dom',
     ),
+    '7ccc4153' => array(
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-install',
+      'javelin-workflow',
+      'javelin-router',
+      'javelin-behavior-device',
+      'javelin-vector',
+      'phabricator-diff-inline',
+    ),
     '80bff3af' => array(
       'javelin-install',
       'javelin-typeahead-source',
@@ -1805,17 +1816,6 @@ return array(
       'javelin-workflow',
       'phabricator-draggable-list',
     ),
-    'a31ffc00' => array(
-      'javelin-dom',
-      'javelin-util',
-      'javelin-stratcom',
-      'javelin-install',
-      'javelin-workflow',
-      'javelin-router',
-      'javelin-behavior-device',
-      'javelin-vector',
-      'phabricator-diff-inline',
-    ),
     'a4356cde' => array(
       'javelin-install',
       'javelin-dom',

src/applications/differential/view/DifferentialChangesetDetailView.php

@@ -13,6 +13,8 @@ final class DifferentialChangesetDetailView extends AphrontView {
   private $autoload;
   private $loaded;
   private $renderer;
+  private $repository;
+  private $diff;
 
   public function setAutoload($autoload) {
     $this->autoload = $autoload;
@@ -196,6 +198,9 @@ final class DifferentialChangesetDetailView extends AphrontView {
           'path' => $display_filename,
           'icon' => $display_icon,
           'treeNodeID' => 'tree-node-'.$changeset->getAnchorName(),
+
+          'editorURI' => $this->getEditorURI(),
+          'editorConfigureURI' => $this->getEditorConfigureURI(),
         ),
         'class' => $class,
         'id'    => $id,
@@ -224,5 +229,59 @@ final class DifferentialChangesetDetailView extends AphrontView {
       ));
   }
 
+  public function setRepository(PhabricatorRepository $repository) {
+    $this->repository = $repository;
+    return $this;
+  }
+
+  public function getRepository() {
+    return $this->repository;
+  }
+
+  public function getChangeset() {
+    return $this->changeset;
+  }
+
+  public function setDiff(DifferentialDiff $diff) {
+    $this->diff = $diff;
+    return $this;
+  }
+
+  public function getDiff() {
+    return $this->diff;
+  }
+
+  private function getEditorURI() {
+    $viewer = $this->getViewer();
+
+    if (!$viewer->isLoggedIn()) {
+      return null;
+    }
+
+    $repository = $this->getRepository();
+    if (!$repository) {
+      return null;
+    }
+
+    $changeset = $this->getChangeset();
+    $diff = $this->getDiff();
+
+    $path = $changeset->getAbsoluteRepositoryPath($repository, $diff);
+    $path = ltrim($path, '/');
+
+    $line = idx($changeset->getMetadata(), 'line:first', 1);
+
+    return $viewer->loadEditorLink($path, $line, $repository);
+  }
+
+  private function getEditorConfigureURI() {
+    $viewer = $this->getViewer();
+
+    if (!$viewer->isLoggedIn()) {
+      return null;
+    }
+
+    return '/settings/panel/display/';
+  }
 
 }

src/applications/differential/view/DifferentialChangesetListView.php

@@ -79,11 +79,19 @@ final class DifferentialChangesetListView extends AphrontView {
     return $this;
   }
 
+  public function getRepository() {
+    return $this->repository;
+  }
+
   public function setDiff(DifferentialDiff $diff) {
     $this->diff = $diff;
     return $this;
   }
 
+  public function getDiff() {
+    return $this->diff;
+  }
+
   public function setRenderingReferences(array $references) {
     $this->references = $references;
     return $this;
@@ -148,6 +156,9 @@ final class DifferentialChangesetListView extends AphrontView {
     $renderer = DifferentialChangesetParser::getDefaultRendererForViewer(
       $viewer);
 
+    $repository = $this->getRepository();
+    $diff = $this->getDiff();
+
     $output = array();
     $ids = array();
     foreach ($changesets as $key => $changeset) {
@@ -158,6 +169,14 @@ final class DifferentialChangesetListView extends AphrontView {
       $detail = id(new DifferentialChangesetDetailView())
         ->setUser($viewer);
 
+      if ($repository) {
+        $detail->setRepository($repository);
+      }
+
+      if ($diff) {
+        $detail->setDiff($diff);
+      }
+
       $uniq_id = 'diff-'.$changeset->getAnchorName();
       $detail->setID($uniq_id);
 
@@ -310,6 +329,15 @@ final class DifferentialChangesetListView extends AphrontView {
 
         'Finish editing inline comments before changing display modes.' =>
           pht('Finish editing inline comments before changing display modes.'),
+
+        'Open file in external editor.' =>
+          pht('Open file in external editor.'),
+
+        'You must select a file to edit.' =>
+          pht('You must select a file to edit.'),
+
+        'No external editor is configured.' =>
+          pht('No external editor is configured.'),
       ),
     ));
 
@@ -388,19 +416,6 @@ final class DifferentialChangesetListView extends AphrontView {
       }
     }
 
-    if ($viewer && $repository) {
-      $path = ltrim(
-        $changeset->getAbsoluteRepositoryPath($repository, $this->diff),
-        '/');
-      $line = idx($changeset->getMetadata(), 'line:first', 1);
-      $editor_link = $viewer->loadEditorLink($path, $line, $repository);
-      if ($editor_link) {
-        $meta['editor'] = $editor_link;
-      } else {
-        $meta['editorConfigure'] = '/settings/panel/display/';
-      }
-    }
-
     $meta['containerID'] = $detail->getID();
 
     return id(new PHUIButtonView())

webroot/rsrc/js/application/diff/DiffChangeset.js

@@ -35,6 +35,9 @@ JX.install('DiffChangeset', {
     this._displayPath = JX.$H(data.displayPath);
     this._icon = data.icon;
 
+    this._editorURI = data.editorURI;
+    this._editorConfigureURI = data.editorConfigureURI;
+
     this._inlines = [];
   },
 
@@ -65,6 +68,17 @@ JX.install('DiffChangeset', {
     _icon: null,
     _treeNodeID: null,
 
+    _editorURI: null,
+    _editorConfigureURI: null,
+
+    getEditorURI: function() {
+      return this._editorURI;
+    },
+
+    getEditorConfigureURI: function() {
+      return this._editorConfigureURI;
+    },
+
     getLeftChangesetID: function() {
       return this._leftID;
     },

webroot/rsrc/js/application/diff/DiffChangesetList.js

@@ -211,6 +211,9 @@ JX.install('DiffChangesetList', {
       label = pht('Hide or show all inline comments.');
       this._installKey('A', label, this._onkeyhideall);
 
+      label = pht('Open file in external editor.');
+      this._installKey('\\', label, this._onkeyopeneditor);
+
     },
 
     isAsleep: function() {
@@ -450,6 +453,29 @@ JX.install('DiffChangesetList', {
       this._warnUser(pht('You must select a file to hide or show.'));
     },
 
+    _onkeyopeneditor: function() {
+      var pht = this.getTranslations();
+      var cursor = this._cursorItem;
+
+      if (cursor) {
+        if (cursor.type == 'file') {
+          var changeset = cursor.changeset;
+          var editor_uri = changeset.getEditorURI();
+
+          if (editor_uri === null) {
+            this._warnUser(pht('No external editor is configured.'));
+            return;
+          }
+
+          JX.$U(editor_uri).go();
+
+          return;
+        }
+      }
+
+      this._warnUser(pht('You must select a file to edit.'));
+    },
+
     _onkeycollapse: function() {
       var cursor = this._cursorItem;
 
@@ -849,8 +875,16 @@ JX.install('DiffChangesetList', {
 
       add_link('fa-arrow-left', pht('Show Raw File (Left)'), data.leftURI);
       add_link('fa-arrow-right', pht('Show Raw File (Right)'), data.rightURI);
-      add_link('fa-pencil', pht('Open in Editor'), data.editor, true);
+
-      add_link('fa-wrench', pht('Configure Editor'), data.editorConfigure);
+      var editor_uri = changeset.getEditorURI();
+      if (editor_uri !== null) {
+        add_link('fa-pencil', pht('Open in Editor'), editor_uri, true);
+      } else {
+        var configure_uri = changeset.getEditorConfigureURI();
+        if (configure_uri !== null) {
+          add_link('fa-wrench', pht('Configure Editor'), configure_uri);
+        }
+      }
 
       menu.setContent(list.getNode());