Page MenuHomePhabricator
Files F15450958

D21282.id50678.diff
No OneTemporary
Actions

D21282.id50678.diff
View Options

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -13,7 +13,7 @@
'core.pkg.js' => '845355f4',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => '5c459f92',
- 'differential.pkg.js' => '24616785',
+ 'differential.pkg.js' => 'a7171fb6',
'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => 'a98c0bf7',
'maniphest.pkg.css' => '35995d6d',
@@ -379,14 +379,15 @@
'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' => '6e5e03d2',
- 'rsrc/js/application/diff/DiffChangesetList.js' => 'b51ba93a',
+ 'rsrc/js/application/diff/DiffChangeset.js' => '3a1ca35b',
+ 'rsrc/js/application/diff/DiffChangesetList.js' => 'cc2c5de5',
'rsrc/js/application/diff/DiffInline.js' => '008b6a15',
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
'rsrc/js/application/differential/behavior-populate.js' => 'b86ef6c2',
'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => '94243d89',
+ 'rsrc/js/application/diffusion/ExternalEditorLinkEngine.js' => '48a8641f',
'rsrc/js/application/diffusion/behavior-audit-preview.js' => 'b7b73831',
'rsrc/js/application/diffusion/behavior-commit-branches.js' => '4b671572',
'rsrc/js/application/diffusion/behavior-commit-graph.js' => 'ef836bf2',
@@ -484,7 +485,7 @@
'rsrc/js/core/behavior-keyboard-pager.js' => '1325b731',
'rsrc/js/core/behavior-keyboard-shortcuts.js' => '42c44e8b',
'rsrc/js/core/behavior-lightbox-attachments.js' => 'c7e748bf',
- 'rsrc/js/core/behavior-line-linker.js' => '590e6527',
+ 'rsrc/js/core/behavior-line-linker.js' => '0d915ff5',
'rsrc/js/core/behavior-linked-container.js' => '74446546',
'rsrc/js/core/behavior-more.js' => '506aa3f4',
'rsrc/js/core/behavior-object-selector.js' => '98ef467f',
@@ -645,7 +646,7 @@
'javelin-behavior-phabricator-gesture-example' => '242dedd0',
'javelin-behavior-phabricator-keyboard-pager' => '1325b731',
'javelin-behavior-phabricator-keyboard-shortcuts' => '42c44e8b',
- 'javelin-behavior-phabricator-line-linker' => '590e6527',
+ 'javelin-behavior-phabricator-line-linker' => '0d915ff5',
'javelin-behavior-phabricator-notification-example' => '29819b75',
'javelin-behavior-phabricator-object-selector' => '98ef467f',
'javelin-behavior-phabricator-oncopy' => 'da8f5259',
@@ -709,6 +710,7 @@
'javelin-dom' => '94681e22',
'javelin-dynval' => '202a2e85',
'javelin-event' => 'c03f2fb4',
+ 'javelin-external-editor-link-engine' => '48a8641f',
'javelin-fx' => '34450586',
'javelin-history' => '030b4f7a',
'javelin-install' => '5902260c',
@@ -774,8 +776,8 @@
'phabricator-darklog' => '3b869402',
'phabricator-darkmessage' => '26cd4b73',
'phabricator-dashboard-css' => '5a205b9d',
- 'phabricator-diff-changeset' => '6e5e03d2',
- 'phabricator-diff-changeset-list' => 'b51ba93a',
+ 'phabricator-diff-changeset' => '3a1ca35b',
+ 'phabricator-diff-changeset-list' => 'cc2c5de5',
'phabricator-diff-inline' => '008b6a15',
'phabricator-diff-path-view' => '8207abf9',
'phabricator-diff-tree-view' => '5d83623b',
@@ -1013,6 +1015,13 @@
'0d2490ce' => array(
'javelin-install',
),
+ '0d915ff5' => array(
+ 'javelin-behavior',
+ 'javelin-stratcom',
+ 'javelin-dom',
+ 'javelin-history',
+ 'javelin-external-editor-link-engine',
+ ),
'0eaa33a9' => array(
'javelin-behavior',
'javelin-dom',
@@ -1222,6 +1231,20 @@
'trigger-rule',
'trigger-rule-type',
),
+ '3a1ca35b' => array(
+ 'javelin-dom',
+ 'javelin-util',
+ 'javelin-stratcom',
+ 'javelin-install',
+ 'javelin-workflow',
+ 'javelin-router',
+ 'javelin-behavior-device',
+ 'javelin-vector',
+ 'phabricator-diff-inline',
+ 'phabricator-diff-path-view',
+ 'phuix-button-view',
+ 'javelin-external-editor-link-engine',
+ ),
'3ae89b20' => array(
'phui-workcard-view-css',
),
@@ -1306,6 +1329,9 @@
'javelin-dom',
'phabricator-draggable-list',
),
+ '48a8641f' => array(
+ 'javelin-install',
+ ),
'48fe33d0' => array(
'javelin-behavior',
'javelin-dom',
@@ -1442,12 +1468,6 @@
'javelin-util',
'javelin-magical-init',
),
- '590e6527' => array(
- 'javelin-behavior',
- 'javelin-stratcom',
- 'javelin-dom',
- 'javelin-history',
- ),
'5a6f6a06' => array(
'javelin-behavior',
'javelin-quicksand',
@@ -1551,19 +1571,6 @@
'javelin-install',
'javelin-util',
),
- '6e5e03d2' => array(
- 'javelin-dom',
- 'javelin-util',
- 'javelin-stratcom',
- 'javelin-install',
- 'javelin-workflow',
- 'javelin-router',
- 'javelin-behavior-device',
- 'javelin-vector',
- 'phabricator-diff-inline',
- 'phabricator-diff-path-view',
- 'phuix-button-view',
- ),
70245195 => array(
'javelin-behavior',
'javelin-stratcom',
@@ -1970,11 +1977,6 @@
'b517bfa0' => array(
'phui-oi-list-view-css',
),
- 'b51ba93a' => array(
- 'javelin-install',
- 'phuix-button-view',
- 'phabricator-diff-tree-view',
- ),
'b557770a' => array(
'javelin-install',
'javelin-util',
@@ -2082,6 +2084,11 @@
'phuix-icon-view',
'phabricator-busy',
),
+ 'cc2c5de5' => array(
+ 'javelin-install',
+ 'phuix-button-view',
+ 'phabricator-diff-tree-view',
+ ),
'cef53b3e' => array(
'javelin-install',
'javelin-dom',
@@ -2422,6 +2429,7 @@
'phuix-formation-view',
'phuix-formation-column-view',
'phuix-formation-flank-view',
+ 'javelin-external-editor-link-engine',
),
'diffusion.pkg.css' => array(
'diffusion-icons-css',
diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php
--- a/resources/celerity/packages.php
+++ b/resources/celerity/packages.php
@@ -220,6 +220,8 @@
'phuix-formation-view',
'phuix-formation-column-view',
'phuix-formation-flank-view',
+
+ 'javelin-external-editor-link-engine',
),
'diffusion.pkg.css' => array(
'diffusion-icons-css',
diff --git a/src/applications/differential/view/DifferentialChangesetDetailView.php b/src/applications/differential/view/DifferentialChangesetDetailView.php
--- a/src/applications/differential/view/DifferentialChangesetDetailView.php
+++ b/src/applications/differential/view/DifferentialChangesetDetailView.php
@@ -252,7 +252,7 @@
'isLowImportance' => $changeset->getIsLowImportanceChangeset(),
'isOwned' => $changeset->getIsOwnedChangeset(),
- 'editorURI' => $this->getEditorURI(),
+ 'editorURITemplate' => $this->getEditorURITemplate(),
'editorConfigureURI' => $this->getEditorConfigureURI(),
'loaded' => $is_loaded,
@@ -321,7 +321,7 @@
return $this->diff;
}
- private function getEditorURI() {
+ private function getEditorURITemplate() {
$repository = $this->getRepository();
if (!$repository) {
return null;
@@ -342,9 +342,7 @@
$path = $changeset->getAbsoluteRepositoryPath($repository, $diff);
$path = ltrim($path, '/');
- $line = idx($changeset->getMetadata(), 'line:first', 1);
-
- return $link_engine->getURIForPath($path, $line);
+ return $link_engine->getURITokensForPath($path);
}
private function getEditorConfigureURI() {
diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js
--- a/webroot/rsrc/js/application/diff/DiffChangeset.js
+++ b/webroot/rsrc/js/application/diff/DiffChangeset.js
@@ -11,6 +11,7 @@
* phabricator-diff-inline
* phabricator-diff-path-view
* phuix-button-view
+ * javelin-external-editor-link-engine
* @javelin
*/
@@ -33,7 +34,7 @@
this._pathParts = data.pathParts;
this._icon = data.icon;
- this._editorURI = data.editorURI;
+ this._editorURITemplate = data.editorURITemplate;
this._editorConfigureURI = data.editorConfigureURI;
this._showPathURI = data.showPathURI;
this._showDirectoryURI = data.showDirectoryURI;
@@ -87,7 +88,7 @@
_changesetList: null,
_icon: null,
- _editorURI: null,
+ _editorURITemplate: null,
_editorConfigureURI: null,
_showPathURI: null,
_showDirectoryURI: null,
@@ -102,8 +103,8 @@
_isSelected: false,
_viewMenu: null,
- getEditorURI: function() {
- return this._editorURI;
+ getEditorURITemplate: function() {
+ return this._editorURITemplate;
},
getEditorConfigureURI: function() {
diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js
--- a/webroot/rsrc/js/application/diff/DiffChangesetList.js
+++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js
@@ -355,48 +355,11 @@
// lines) reply on the old file.
if (cursor.type == 'change') {
- var origin = cursor.nodes.begin;
- var target = cursor.nodes.end;
-
- // The "origin" and "target" are entire rows, but we need to find
- // a range of "<th />" nodes to actually create an inline, so go
- // fishing.
-
- var old_list = [];
- var new_list = [];
-
- var row = origin;
- while (row) {
- var header = row.firstChild;
- while (header) {
- if (this.getLineNumberFromHeader(header)) {
- if (header.className.indexOf('old') !== -1) {
- old_list.push(header);
- } else if (header.className.indexOf('new') !== -1) {
- new_list.push(header);
- }
- }
- header = header.nextSibling;
- }
-
- if (row == target) {
- break;
- }
-
- row = row.nextSibling;
- }
-
- var use_list;
- if (new_list.length) {
- use_list = new_list;
- } else {
- use_list = old_list;
- }
+ var cells = this._getLineNumberCellsForChangeBlock(
+ cursor.nodes.begin,
+ cursor.nodes.end);
- var src = use_list[0];
- var dst = use_list[use_list.length - 1];
-
- cursor.changeset.newInlineForRange(src, dst);
+ cursor.changeset.newInlineForRange(cells.src, cells.dst);
this.setFocus(null);
return;
@@ -407,6 +370,51 @@
this._warnUser(pht('You must select a comment or change to reply to.'));
},
+ _getLineNumberCellsForChangeBlock: function(origin, target) {
+ // The "origin" and "target" are entire rows, but we need to find
+ // a range of cell nodes to actually create an inline, so go
+ // fishing.
+
+ var old_list = [];
+ var new_list = [];
+
+ var row = origin;
+ while (row) {
+ var header = row.firstChild;
+ while (header) {
+ if (this.getLineNumberFromHeader(header)) {
+ if (header.className.indexOf('old') !== -1) {
+ old_list.push(header);
+ } else if (header.className.indexOf('new') !== -1) {
+ new_list.push(header);
+ }
+ }
+ header = header.nextSibling;
+ }
+
+ if (row == target) {
+ break;
+ }
+
+ row = row.nextSibling;
+ }
+
+ var use_list;
+ if (new_list.length) {
+ use_list = new_list;
+ } else {
+ use_list = old_list;
+ }
+
+ var src = use_list[0];
+ var dst = use_list[use_list.length - 1];
+
+ return {
+ src: src,
+ dst: dst
+ };
+ },
+
_onkeyedit: function() {
var cursor = this._cursorItem;
@@ -505,7 +513,7 @@
return changeset;
},
- _onkeyopeneditor: function() {
+ _onkeyopeneditor: function(e) {
var pht = this.getTranslations();
var changeset = this._getChangesetForKeyCommand();
@@ -514,13 +522,61 @@
return;
}
- var editor_uri = changeset.getEditorURI();
+ this._openEditor(changeset);
+ },
+
+ _openEditor: function(changeset) {
+ var pht = this.getTranslations();
- if (editor_uri === null) {
+ var editor_template = changeset.getEditorURITemplate();
+ if (editor_template === null) {
this._warnUser(pht('No external editor is configured.'));
return;
}
+ var line = null;
+
+ // See PHI1749. We aren't exactly sure what the user intends when they
+ // use the keyboard to select a change block and then activate the
+ // "Open in Editor" function: they might mean to open the old or new
+ // offset, and may have the old or new state (or some other state) in
+ // their working copy.
+
+ // For now, pick: the new state line number if one exists; or the old
+ // state line number if one does not. If nothing else, this behavior is
+ // simple.
+
+ // If there's a document engine, just open the file to the first line.
+ // We currently can not map display blocks to source lines.
+
+ // If there's an inline, open the file to that line.
+
+ if (changeset.getResponseDocumentEngineKey() === null) {
+ var cursor = this._cursorItem;
+ if (cursor && (cursor.changeset === changeset)) {
+ if (cursor.type == 'change') {
+ var cells = this._getLineNumberCellsForChangeBlock(
+ cursor.nodes.begin,
+ cursor.nodes.end);
+ line = this.getLineNumberFromHeader(cells.src);
+ }
+
+ if (cursor.type === 'comment') {
+ var inline = cursor.target;
+ line = inline.getLineNumber();
+ }
+ }
+ }
+
+ var variables = {
+ l: line || 1
+ };
+
+ var editor_uri = new JX.ExternalEditorLinkEngine()
+ .setTemplate(editor_template)
+ .setVariables(variables)
+ .newURI();
+
JX.$U(editor_uri).go();
},
@@ -1029,10 +1085,21 @@
changeset.getShowPathURI())
.setKeyCommand('d');
- var editor_uri = changeset.getEditorURI();
- if (editor_uri !== null) {
- add_link('fa-i-cursor', pht('Open in Editor'), editor_uri, true)
- .setKeyCommand('\\');
+ var editor_template = changeset.getEditorURITemplate();
+ if (editor_template !== null) {
+ var editor_item = new JX.PHUIXActionView()
+ .setIcon('fa-i-cursor')
+ .setName(pht('Open in Editor'))
+ .setKeyCommand('\\')
+ .setHandler(function(e) {
+
+ changeset_list._openEditor(changeset);
+
+ e.prevent();
+ menu.close();
+ });
+
+ list.addItem(editor_item);
} else {
var configure_uri = changeset.getEditorConfigureURI();
if (configure_uri !== null) {
diff --git a/webroot/rsrc/js/application/diffusion/ExternalEditorLinkEngine.js b/webroot/rsrc/js/application/diffusion/ExternalEditorLinkEngine.js
new file mode 100644
--- /dev/null
+++ b/webroot/rsrc/js/application/diffusion/ExternalEditorLinkEngine.js
@@ -0,0 +1,41 @@
+/**
+ * @provides javelin-external-editor-link-engine
+ * @requires javelin-install
+ * @javelin
+ */
+
+JX.install('ExternalEditorLinkEngine', {
+
+ properties: {
+ template: null,
+ variables: null
+ },
+
+ members: {
+ newURI: function() {
+ var template = this.getTemplate();
+ var variables = this.getVariables();
+
+ var parts = [];
+ for (var ii = 0; ii < template.length; ii++) {
+ var part = template[ii];
+ var value = part.value;
+
+ if (part.type === 'literal') {
+ parts.push(value);
+ continue;
+ }
+
+ if (part.type === 'variable') {
+ if (variables.hasOwnProperty(value)) {
+ var replacement = variables[value];
+ replacement = encodeURIComponent(replacement);
+ parts.push(replacement);
+ }
+ }
+ }
+
+ return parts.join('');
+ }
+ }
+});
diff --git a/webroot/rsrc/js/core/behavior-line-linker.js b/webroot/rsrc/js/core/behavior-line-linker.js
--- a/webroot/rsrc/js/core/behavior-line-linker.js
+++ b/webroot/rsrc/js/core/behavior-line-linker.js
@@ -4,6 +4,7 @@
* javelin-stratcom
* javelin-dom
* javelin-history
+ * javelin-external-editor-link-engine
*/
JX.behavior('phabricator-line-linker', function() {
@@ -170,32 +171,19 @@
if (editor_link) {
var data = JX.Stratcom.getData(editor_link);
- var template = data.template;
var variables = {
l: parseInt(Math.min(o, t), 10),
};
- var parts = [];
- for (var ii = 0; ii < template.length; ii++) {
- var part = template[ii];
- var value = part.value;
-
- if (part.type === 'literal') {
- parts.push(value);
- continue;
- }
-
- if (part.type === 'variable') {
- if (variables.hasOwnProperty(value)) {
- var replacement = variables[value];
- replacement = encodeURIComponent(replacement);
- parts.push(replacement);
- }
- }
- }
-
- editor_link.href = parts.join('');
+ var template = data.template;
+
+ var editor_uri = new JX.ExternalEditorLinkEngine()
+ .setTemplate(template)
+ .setVariables(variables)
+ .newURI();
+
+ editor_link.href = editor_uri;
}
});

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 29, 7:16 PM (3 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7309368
Default Alt Text
D21282.id50678.diff (17 KB)

Event Timeline

Phabricator · Built by Phacility