Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15409948
D17920.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
14 KB
Referenced Files
None
Subscribers
None
D17920.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' => '8c5f913d',
'darkconsole.pkg.js' => '1f9a31bc',
'differential.pkg.css' => 'ea471cb0',
- 'differential.pkg.js' => '7f24021f',
+ 'differential.pkg.js' => 'ae6f5198',
'diffusion.pkg.css' => 'b93d9b8c',
'diffusion.pkg.js' => '84c8f8fd',
'favicon.ico' => '30672e08',
@@ -390,13 +390,13 @@
'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '408bf173',
'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375',
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63',
- 'rsrc/js/application/diff/DiffChangeset.js' => '34e513e2',
- 'rsrc/js/application/diff/DiffChangesetList.js' => 'fcbf80e9',
- 'rsrc/js/application/diff/DiffInline.js' => 'c2e9ff4c',
+ 'rsrc/js/application/diff/DiffChangeset.js' => '68758d99',
+ 'rsrc/js/application/diff/DiffChangesetList.js' => '57c491b5',
+ 'rsrc/js/application/diff/DiffInline.js' => '1afe9760',
'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76',
'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1',
- 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => 'd59300f5',
+ 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '1c6bc8cf',
'rsrc/js/application/differential/behavior-populate.js' => '5e41c819',
'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb',
'rsrc/js/application/differential/behavior-user-select.js' => 'a8d8459d',
@@ -619,7 +619,7 @@
'javelin-behavior-device' => 'bb1dd507',
'javelin-behavior-diff-preview-link' => '051c7832',
'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
- 'javelin-behavior-differential-edit-inline-comments' => 'd59300f5',
+ 'javelin-behavior-differential-edit-inline-comments' => '1c6bc8cf',
'javelin-behavior-differential-feedback-preview' => 'b064af76',
'javelin-behavior-differential-populate' => '5e41c819',
'javelin-behavior-differential-toggle-files' => 'ca3f91eb',
@@ -779,9 +779,9 @@
'phabricator-darklog' => 'c8e1ffe3',
'phabricator-darkmessage' => 'c48cccdd',
'phabricator-dashboard-css' => 'fe5b1869',
- 'phabricator-diff-changeset' => '34e513e2',
- 'phabricator-diff-changeset-list' => 'fcbf80e9',
- 'phabricator-diff-inline' => 'c2e9ff4c',
+ 'phabricator-diff-changeset' => '68758d99',
+ 'phabricator-diff-changeset-list' => '57c491b5',
+ 'phabricator-diff-inline' => '1afe9760',
'phabricator-drag-and-drop-file-upload' => '58dea2fa',
'phabricator-draggable-list' => 'bea6e7f4',
'phabricator-fatal-config-template-css' => '8f18fa41',
@@ -1030,6 +1030,9 @@
'javelin-util',
'phabricator-keyboard-shortcut-manager',
),
+ '1afe9760' => array(
+ 'javelin-dom',
+ ),
'1bd28176' => array(
'javelin-install',
'javelin-dom',
@@ -1037,6 +1040,13 @@
'javelin-request',
'javelin-uri',
),
+ '1c6bc8cf' => array(
+ 'javelin-behavior',
+ 'javelin-stratcom',
+ 'javelin-dom',
+ 'javelin-util',
+ 'javelin-vector',
+ ),
'1def2711' => array(
'javelin-install',
'javelin-dom',
@@ -1129,17 +1139,6 @@
'javelin-dom',
'javelin-workflow',
),
- '34e513e2' => array(
- 'javelin-dom',
- 'javelin-util',
- 'javelin-stratcom',
- 'javelin-install',
- 'javelin-workflow',
- 'javelin-router',
- 'javelin-behavior-device',
- 'javelin-vector',
- 'phabricator-diff-inline',
- ),
'3ab51e2c' => array(
'javelin-behavior',
'javelin-behavior-device',
@@ -1332,6 +1331,9 @@
'javelin-vector',
'javelin-dom',
),
+ '57c491b5' => array(
+ 'javelin-install',
+ ),
'58dea2fa' => array(
'javelin-install',
'javelin-util',
@@ -1406,6 +1408,17 @@
'javelin-dom',
'phabricator-notification',
),
+ '68758d99' => array(
+ 'javelin-dom',
+ 'javelin-util',
+ 'javelin-stratcom',
+ 'javelin-install',
+ 'javelin-workflow',
+ 'javelin-router',
+ 'javelin-behavior-device',
+ 'javelin-vector',
+ 'phabricator-diff-inline',
+ ),
'6882e80a' => array(
'javelin-dom',
),
@@ -1910,9 +1923,6 @@
'javelin-dom',
'javelin-vector',
),
- 'c2e9ff4c' => array(
- 'javelin-dom',
- ),
'c420b0b9' => array(
'javelin-behavior',
'javelin-behavior-device',
@@ -2036,13 +2046,6 @@
'javelin-dom',
'javelin-stratcom',
),
- 'd59300f5' => array(
- 'javelin-behavior',
- 'javelin-stratcom',
- 'javelin-dom',
- 'javelin-util',
- 'javelin-vector',
- ),
'd5a2d665' => array(
'javelin-behavior',
'javelin-stratcom',
@@ -2221,9 +2224,6 @@
'javelin-dom',
'phortune-credit-card-form',
),
- 'fcbf80e9' => array(
- 'javelin-install',
- ),
'fe287620' => array(
'javelin-install',
'javelin-dom',
diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php
--- a/src/applications/differential/view/DifferentialChangesetListView.php
+++ b/src/applications/differential/view/DifferentialChangesetListView.php
@@ -244,15 +244,19 @@
pht('Jump to previous inline comment.'),
'Jump to the table of contents.' =>
pht('Jump to the table of contents.'),
- 'Reply to selected inline comment.' =>
- pht('Reply to selected inline comment.'),
+
'Edit selected inline comment.' =>
pht('Edit selected inline comment.'),
- 'You must select a comment to reply to.' =>
- pht('You must select a comment to reply to.'),
'You must select a comment to edit.' =>
pht('You must select a comment to edit.'),
+ 'Reply to selected inline comment or change.' =>
+ pht('Reply to selected inline comment or change.'),
+ 'You must select a comment or change to reply to.' =>
+ pht('You must select a comment or change to reply to.'),
+ 'Reply and quote selected inline comment.' =>
+ pht('Reply and quote selected inline comment.'),
+
'Mark or unmark selected inline comment as done.' =>
pht('Mark or unmark selected inline comment as done.'),
'You must select a comment to mark done.' =>
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
@@ -526,7 +526,37 @@
return data.inline;
},
- newInlineForRange: function(data) {
+ newInlineForRange: function(origin, target) {
+ var list = this.getChangesetList();
+
+ var src = list.getLineNumberFromHeader(origin);
+ var dst = list.getLineNumberFromHeader(target);
+
+ var changeset_id = null;
+ var side = list.getDisplaySideFromHeader(origin);
+ if (side == 'right') {
+ changeset_id = this.getRightChangesetID();
+ } else {
+ changeset_id = this.getLeftChangesetID();
+ }
+
+ var is_new = false;
+ if (side == 'right') {
+ is_new = true;
+ } else if (this.getRightChangesetID() != this.getLeftChangesetID()) {
+ is_new = true;
+ }
+
+ var data = {
+ origin: origin,
+ target: target,
+ number: src,
+ length: dst - src,
+ changesetID: changeset_id,
+ displaySide: side,
+ isNewFile: is_new
+ };
+
var inline = new JX.DiffInline()
.setChangeset(this)
.bindToRange(data);
@@ -538,14 +568,14 @@
return inline;
},
- newInlineReply: function(original) {
+ newInlineReply: function(original, text) {
var inline = new JX.DiffInline()
.setChangeset(this)
.bindToReply(original);
this._inlines.push(inline);
- inline.create();
+ inline.create(text);
return inline;
},
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
@@ -136,8 +136,11 @@
label = pht('Jump to the table of contents.');
this._installKey('t', label, this._ontoc);
- label = pht('Reply to selected inline comment.');
- this._installKey('r', label, this._onkeyreply);
+ label = pht('Reply to selected inline comment or change.');
+ this._installKey('r', label, JX.bind(this, this._onkeyreply, false));
+
+ label = pht('Reply and quote selected inline comment.');
+ this._installKey('R', label, JX.bind(this, this._onkeyreply, true));
label = pht('Edit selected inline comment.');
this._installKey('e', label, this._onkeyedit);
@@ -234,7 +237,7 @@
manager.scrollTo(toc);
},
- _onkeyreply: function() {
+ _onkeyreply: function(is_quote) {
var cursor = this._cursorItem;
if (cursor) {
@@ -243,14 +246,77 @@
if (inline.canReply()) {
this.setFocus(null);
- inline.reply();
+ var text;
+ if (is_quote) {
+ text = inline.getRawText();
+ text = '> ' + text.replace(/\n/g, '\n> ') + '\n\n';
+ } else {
+ text = '';
+ }
+
+ inline.reply(text);
return;
}
}
+
+ // If the keyboard cursor is selecting a range of lines, we may have
+ // a mixture of old and new changes on the selected rows. It is not
+ // entirely unambiguous what the user means when they say they want
+ // to reply to this, but we use this logic: reply on the new file if
+ // there are any new lines. Otherwise (if there are only removed
+ // 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 (JX.DOM.isType(header, 'th')) {
+ 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];
+
+ cursor.changeset.newInlineForRange(src, dst);
+
+ this.setFocus(null);
+ return;
+ }
}
var pht = this.getTranslations();
- this._warnUser(pht('You must select a comment to reply to.'));
+ this._warnUser(pht('You must select a comment or change to reply to.'));
},
_onkeyedit: function() {
@@ -804,7 +870,7 @@
// Clear the mouse hover reticle after a substantive edit: we don't get
// a "mouseout" event if the row vanished because of row being removed
// after an edit.
- this.reseHover();
+ this.resetHover();
},
setFocus: function(node, extended_node) {
@@ -978,8 +1044,19 @@
var inline_row = e.getNode('inline-row');
return changeset.getInlineForRow(inline_row);
- }
+ },
+
+ getLineNumberFromHeader: function(th) {
+ try {
+ return parseInt(th.id.match(/^C\d+[ON]L(\d+)$/)[1], 10);
+ } catch (x) {
+ return null;
+ }
+ },
+ getDisplaySideFromHeader: function(th) {
+ return (th.parentNode.firstChild != th) ? 'right' : 'left';
+ }
}
});
diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js
--- a/webroot/rsrc/js/application/diff/DiffInline.js
+++ b/webroot/rsrc/js/application/diff/DiffInline.js
@@ -148,6 +148,10 @@
return true;
},
+ getRawText: function() {
+ return this._originalText;
+ },
+
_hasAction: function(action) {
var nodes = JX.DOM.scry(this._row, 'a', 'differential-inline-' + action);
return (nodes.length > 0);
@@ -247,9 +251,9 @@
.send();
},
- reply: function() {
+ reply: function(text) {
var changeset = this.getChangeset();
- return changeset.newInlineReply(this);
+ return changeset.newInlineReply(this, text);
},
edit: function(text) {
@@ -365,13 +369,9 @@
},
_oncreateresponse: function(response) {
- try {
var rows = JX.$H(response).getNode();
this._drawEditRows(rows);
- } catch (e) {
- JX.log(e);
- }
},
_ondeleteresponse: function() {
@@ -471,7 +471,11 @@
'textarea',
'differential-inline-comment-edit-textarea');
if (textareas.length) {
- textareas[0].focus();
+ var area = textareas[0];
+ area.focus();
+
+ var length = area.value.length;
+ JX.TextAreaUtils.setSelectionRange(area, length, length);
}
row = next_row;
diff --git a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js
--- a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js
+++ b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js
@@ -72,11 +72,6 @@
return node.parentNode.firstChild != node;
}
- function isNewFile(node) {
- var data = JX.Stratcom.getData(root);
- return isOnRight(node) || (data.left != data.right);
- }
-
function getRowNumber(th_node) {
try {
return parseInt(th_node.id.match(/^C\d+[ON]L(\d+)$/)[1], 10);
@@ -192,15 +187,7 @@
var view = JX.DiffChangeset.getForNode(root);
- view.newInlineForRange({
- origin: origin,
- target: target,
- number: o,
- length: len,
- changesetID: changeset,
- isNewFile: isNewFile(target),
- displaySide: isOnRight(target) ? 'right' : 'left'
- });
+ view.newInlineForRange(origin, target);
selecting = false;
origin = null;
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, Mar 20, 6:04 AM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7712661
Default Alt Text
D17920.diff (14 KB)
Attached To
Mode
D17920: In Differential, allow "r" to create comments and "R" to quote
Attached
Detach File
Event Timeline
Log In to Comment