Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15458155
D21216.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
20 KB
Referenced Files
None
Subscribers
None
D21216.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' => '632fb8f5',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => '2d70b7b9',
- 'differential.pkg.js' => '4287e51f',
+ 'differential.pkg.js' => '4d375e61',
'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => 'a98c0bf7',
'maniphest.pkg.css' => '35995d6d',
@@ -380,8 +380,8 @@
'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' => 'a49dc31e',
- 'rsrc/js/application/diff/DiffChangesetList.js' => '10726e6a',
- 'rsrc/js/application/diff/DiffInline.js' => '7f804f2b',
+ 'rsrc/js/application/diff/DiffChangesetList.js' => '6992b85c',
+ 'rsrc/js/application/diff/DiffInline.js' => 'a39fd98e',
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
'rsrc/js/application/diff/behavior-preview-link.js' => 'f51e9c17',
@@ -462,7 +462,7 @@
'rsrc/js/core/MultirowRowManager.js' => '5b54c823',
'rsrc/js/core/Notification.js' => 'a9b91e3f',
'rsrc/js/core/Prefab.js' => '5793d835',
- 'rsrc/js/core/ShapedRequest.js' => 'abf88db8',
+ 'rsrc/js/core/ShapedRequest.js' => '995f5102',
'rsrc/js/core/TextAreaUtils.js' => 'f340a484',
'rsrc/js/core/Title.js' => '43bc9360',
'rsrc/js/core/ToolTip.js' => '83754533',
@@ -777,8 +777,8 @@
'phabricator-darkmessage' => '26cd4b73',
'phabricator-dashboard-css' => '5a205b9d',
'phabricator-diff-changeset' => 'a49dc31e',
- 'phabricator-diff-changeset-list' => '10726e6a',
- 'phabricator-diff-inline' => '7f804f2b',
+ 'phabricator-diff-changeset-list' => '6992b85c',
+ 'phabricator-diff-inline' => 'a39fd98e',
'phabricator-diff-path-view' => '8207abf9',
'phabricator-diff-tree-view' => '5d83623b',
'phabricator-drag-and-drop-file-upload' => '4370900d',
@@ -800,7 +800,7 @@
'phabricator-prefab' => '5793d835',
'phabricator-remarkup-css' => 'c286eaef',
'phabricator-search-results-css' => '9ea70ace',
- 'phabricator-shaped-request' => 'abf88db8',
+ 'phabricator-shaped-request' => '995f5102',
'phabricator-slowvote-css' => '1694baed',
'phabricator-source-code-view-css' => '03d7ac28',
'phabricator-standard-page-view' => 'a374f94c',
@@ -1022,11 +1022,6 @@
'javelin-workflow',
'phuix-icon-view',
),
- '10726e6a' => array(
- 'javelin-install',
- 'phuix-button-view',
- 'phabricator-diff-tree-view',
- ),
'111bfd2d' => array(
'javelin-install',
),
@@ -1519,6 +1514,11 @@
'javelin-install',
'javelin-dom',
),
+ '6992b85c' => array(
+ 'javelin-install',
+ 'phuix-button-view',
+ 'phabricator-diff-tree-view',
+ ),
'6a1583a8' => array(
'javelin-behavior',
'javelin-history',
@@ -1626,9 +1626,6 @@
'javelin-install',
'javelin-dom',
),
- '7f804f2b' => array(
- 'javelin-dom',
- ),
'80bff3af' => array(
'javelin-install',
'javelin-typeahead-source',
@@ -1797,6 +1794,12 @@
'javelin-request',
'javelin-util',
),
+ '995f5102' => array(
+ 'javelin-install',
+ 'javelin-util',
+ 'javelin-request',
+ 'javelin-router',
+ ),
'9aae2b66' => array(
'javelin-install',
'javelin-util',
@@ -1838,6 +1841,9 @@
'javelin-workflow',
'phabricator-draggable-list',
),
+ 'a39fd98e' => array(
+ 'javelin-dom',
+ ),
'a4356cde' => array(
'javelin-install',
'javelin-dom',
@@ -1916,12 +1922,6 @@
'javelin-dom',
'phabricator-notification',
),
- 'abf88db8' => array(
- 'javelin-install',
- 'javelin-util',
- 'javelin-request',
- 'javelin-router',
- ),
'ad258e28' => array(
'javelin-behavior',
'javelin-dom',
diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -105,7 +105,13 @@
switch ($xaction->getTransactionType()) {
case PhabricatorAuditActionConstants::INLINE:
- $xaction->getComment()->setAttribute('editing', false);
+ $comment = $xaction->getComment();
+
+ $comment->setAttribute('editing', false);
+
+ PhabricatorVersionedDraft::purgeDrafts(
+ $comment->getPHID(),
+ $this->getActingAsPHID());
return;
case PhabricatorAuditTransaction::TYPE_COMMIT:
return;
diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php
--- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php
+++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php
@@ -111,13 +111,6 @@
$viewer->getPHID());
}
- foreach ($inlines as $key => $inline) {
- $is_draft = !$inline->getTransactionPHID();
- if ($is_draft && $inline->isEmptyInlineComment()) {
- unset($inlines[$key]);
- }
- }
-
return self::buildProxies($inlines);
}
diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php
--- a/src/applications/differential/controller/DifferentialChangesetViewController.php
+++ b/src/applications/differential/controller/DifferentialChangesetViewController.php
@@ -197,7 +197,6 @@
$query = id(new DifferentialInlineCommentQuery())
->setViewer($viewer)
->needHidden(true)
- ->withEmptyInlineComments(false)
->withRevisionPHIDs(array($revision->getPHID()));
$inlines = $query->execute();
$inlines = $query->adjustInlinesForChangesets(
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -112,7 +112,13 @@
switch ($xaction->getTransactionType()) {
case DifferentialTransaction::TYPE_INLINE:
- $xaction->getComment()->setAttribute('editing', false);
+ $comment = $xaction->getComment();
+
+ $comment->setAttribute('editing', false);
+
+ PhabricatorVersionedDraft::purgeDrafts(
+ $comment->getPHID(),
+ $this->getActingAsPHID());
return;
}
diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php
--- a/src/applications/differential/parser/DifferentialChangesetParser.php
+++ b/src/applications/differential/parser/DifferentialChangesetParser.php
@@ -752,6 +752,8 @@
$range_len = null,
$mask_force = array()) {
+ $viewer = $this->getViewer();
+
$renderer = $this->getRenderer();
if (!$renderer) {
$renderer = $this->newRenderer();
@@ -853,6 +855,16 @@
$has_document_engine = ($engine_blocks !== null);
+ // Remove empty comments that don't have any unsaved draft data.
+ PhabricatorInlineComment::loadAndAttachVersionedDrafts(
+ $viewer,
+ $this->comments);
+ foreach ($this->comments as $key => $comment) {
+ if ($comment->isVoidComment($viewer)) {
+ unset($this->comments[$key]);
+ }
+ }
+
// See T13515. Sometimes, we collapse file content by default: for
// example, if the file is marked as containing generated code.
@@ -1050,6 +1062,7 @@
}
}
}
+
$renderer
->setOldComments($old_comments)
->setNewComments($new_comments);
diff --git a/src/applications/differential/query/DifferentialInlineCommentQuery.php b/src/applications/differential/query/DifferentialInlineCommentQuery.php
--- a/src/applications/differential/query/DifferentialInlineCommentQuery.php
+++ b/src/applications/differential/query/DifferentialInlineCommentQuery.php
@@ -16,7 +16,6 @@
private $revisionPHIDs;
private $deletedDrafts;
private $needHidden;
- private $withEmpty;
public function setViewer(PhabricatorUser $viewer) {
$this->viewer = $viewer;
@@ -62,11 +61,6 @@
return $this;
}
- public function withEmptyInlineComments($empty) {
- $this->withEmpty = $empty;
- return $this;
- }
-
public function execute() {
$table = new DifferentialTransactionComment();
$conn_r = $table->establishConnection('r');
@@ -80,15 +74,6 @@
$comments = $table->loadAllFromArray($data);
- if ($this->withEmpty !== null) {
- $want_empty = (bool)$this->withEmpty;
- foreach ($comments as $key => $value) {
- if ($value->isEmptyInlineComment() !== $want_empty) {
- unset($comments[$key]);
- }
- }
- }
-
if ($this->needHidden) {
$viewer_phid = $this->getViewer()->getPHID();
if ($viewer_phid && $comments) {
diff --git a/src/applications/differential/query/DifferentialTransactionQuery.php b/src/applications/differential/query/DifferentialTransactionQuery.php
--- a/src/applications/differential/query/DifferentialTransactionQuery.php
+++ b/src/applications/differential/query/DifferentialTransactionQuery.php
@@ -20,13 +20,24 @@
->needReplyToComments(true)
->execute();
- // Don't count empty inlines when considering draft state.
foreach ($inlines as $key => $inline) {
- if ($inline->isEmptyInlineComment()) {
+ $inlines[$key] = DifferentialInlineComment::newFromModernComment(
+ $inline);
+ }
+
+ PhabricatorInlineComment::loadAndAttachVersionedDrafts(
+ $viewer,
+ $inlines);
+
+ // Don't count void inlines when considering draft state.
+ foreach ($inlines as $key => $inline) {
+ if ($inline->isVoidComment($viewer)) {
unset($inlines[$key]);
}
}
+ $inlines = mpull($inlines, 'getStorageObject');
+
return $inlines;
}
diff --git a/src/applications/draft/storage/PhabricatorVersionedDraft.php b/src/applications/draft/storage/PhabricatorVersionedDraft.php
--- a/src/applications/draft/storage/PhabricatorVersionedDraft.php
+++ b/src/applications/draft/storage/PhabricatorVersionedDraft.php
@@ -35,6 +35,23 @@
return idx($this->properties, $key, $default);
}
+ public static function loadDrafts(
+ array $object_phids,
+ $viewer_phid) {
+
+ $rows = id(new self())->loadAllWhere(
+ 'objectPHID IN (%Ls) AND authorPHID = %s ORDER BY version ASC',
+ $object_phids,
+ $viewer_phid);
+
+ $map = array();
+ foreach ($rows as $row) {
+ $map[$row->getObjectPHID()] = $row;
+ }
+
+ return $map;
+ }
+
public static function loadDraft(
$object_phid,
$viewer_phid) {
diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php
--- a/src/infrastructure/diff/PhabricatorInlineCommentController.php
+++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php
@@ -192,11 +192,15 @@
->setIsEditing(false);
$this->saveComment($inline);
+ $this->purgeVersionedDrafts($inline);
+
return $this->buildRenderedCommentResponse(
$inline,
$this->getIsOnRight());
} else {
$this->deleteComment($inline);
+ $this->purgeVersionedDrafts($inline);
+
return $this->buildEmptyResponse();
}
} else {
@@ -235,6 +239,23 @@
$this->saveComment($inline);
}
+ $this->purgeVersionedDrafts($inline);
+
+ return $this->buildEmptyResponse();
+ case 'draft':
+ $inline = $this->loadCommentForEdit($this->getCommentID());
+
+ $versioned_draft = PhabricatorVersionedDraft::loadOrCreateDraft(
+ $inline->getPHID(),
+ $viewer->getPHID(),
+ $inline->getID());
+
+ $text = $this->getCommentText();
+
+ $versioned_draft
+ ->setProperty('inline.text', $text)
+ ->save();
+
return $this->buildEmptyResponse();
case 'new':
case 'reply':
@@ -405,4 +426,13 @@
->setContent($response);
}
+ private function purgeVersionedDrafts(
+ PhabricatorInlineComment $inline) {
+ $viewer = $this->getViewer();
+ PhabricatorVersionedDraft::purgeDrafts(
+ $inline->getPHID(),
+ $viewer->getPHID());
+ }
+
+
}
diff --git a/src/infrastructure/diff/interface/PhabricatorInlineComment.php b/src/infrastructure/diff/interface/PhabricatorInlineComment.php
--- a/src/infrastructure/diff/interface/PhabricatorInlineComment.php
+++ b/src/infrastructure/diff/interface/PhabricatorInlineComment.php
@@ -15,11 +15,51 @@
private $storageObject;
private $syntheticAuthor;
private $isGhost;
+ private $versionedDrafts = array();
public function __clone() {
$this->storageObject = clone $this->storageObject;
}
+ final public static function loadAndAttachVersionedDrafts(
+ PhabricatorUser $viewer,
+ array $inlines) {
+
+ $viewer_phid = $viewer->getPHID();
+ if (!$viewer_phid) {
+ return;
+ }
+
+ $inlines = mpull($inlines, null, 'getPHID');
+
+ $load = array();
+ foreach ($inlines as $key => $inline) {
+ if (!$inline->getIsEditing()) {
+ continue;
+ }
+
+ if ($inline->getAuthorPHID() !== $viewer_phid) {
+ continue;
+ }
+
+ $load[$key] = $inline;
+ }
+
+ if (!$load) {
+ return;
+ }
+
+ $drafts = PhabricatorVersionedDraft::loadDrafts(
+ array_keys($load),
+ $viewer_phid);
+
+ $drafts = mpull($drafts, null, 'getObjectPHID');
+ foreach ($inlines as $inline) {
+ $draft = idx($drafts, $inline->getPHID());
+ $inline->attachVersionedDraftForViewer($viewer, $draft);
+ }
+ }
+
public function setSyntheticAuthor($synthetic_author) {
$this->syntheticAuthor = $synthetic_author;
return $this;
@@ -204,6 +244,57 @@
return $this;
}
+ public function attachVersionedDraftForViewer(
+ PhabricatorUser $viewer,
+ PhabricatorVersionedDraft $draft = null) {
+
+ $key = $viewer->getCacheFragment();
+ $this->versionedDrafts[$key] = $draft;
+
+ return $this;
+ }
+
+ public function hasVersionedDraftForViewer(PhabricatorUser $viewer) {
+ $key = $viewer->getCacheFragment();
+ return array_key_exists($key, $this->versionedDrafts);
+ }
+
+ public function getVersionedDraftForViewer(PhabricatorUser $viewer) {
+ $key = $viewer->getCacheFragment();
+ if (!array_key_exists($key, $this->versionedDrafts)) {
+ throw new Exception(
+ pht(
+ 'Versioned draft is not attached for user with fragment "%s".',
+ $key));
+ }
+
+ return $this->versionedDrafts[$key];
+ }
+
+ public function isVoidComment(PhabricatorUser $viewer) {
+ return !strlen($this->getContentForEdit($viewer));
+ }
+
+ public function getContentForEdit(PhabricatorUser $viewer) {
+ $content = $this->getContent();
+
+ if (!$this->hasVersionedDraftForViewer($viewer)) {
+ return $content;
+ }
+
+ $versioned_draft = $this->getVersionedDraftForViewer($viewer);
+ if (!$versioned_draft) {
+ return $content;
+ }
+
+ $draft_text = $versioned_draft->getProperty('inline.text');
+ if ($draft_text === null) {
+ return $content;
+ }
+
+ return $draft_text;
+ }
+
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php
--- a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php
+++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php
@@ -54,6 +54,7 @@
}
protected function getInlineCommentMetadata() {
+ $viewer = $this->getViewer();
$inline = $this->getInlineComment();
$is_synthetic = (bool)$inline->getSyntheticAuthor();
@@ -74,6 +75,8 @@
break;
}
+ $original_text = $inline->getContentForEdit($viewer);
+
return array(
'id' => $inline->getID(),
'phid' => $inline->getPHID(),
@@ -81,7 +84,7 @@
'number' => $inline->getLineNumber(),
'length' => $inline->getLineLength(),
'isNewFile' => (bool)$inline->getIsNewFile(),
- 'original' => $inline->getContent(),
+ 'original' => $original_text,
'replyToCommentPHID' => $inline->getReplyToCommentPHID(),
'isDraft' => $inline->isDraft(),
'isFixed' => $is_fixed,
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
@@ -2110,6 +2110,13 @@
'click',
['differential-inline-comment', 'differential-inline-reply'],
onreply);
+
+ var ondraft = JX.bind(this, this._onInlineEvent, 'draft');
+ JX.Stratcom.listen(
+ 'keydown',
+ ['differential-inline-comment', 'tag:textarea'],
+ ondraft);
+
},
_onInlineEvent: function(action, e) {
@@ -2117,7 +2124,9 @@
return;
}
- e.kill();
+ if (action !== 'draft') {
+ e.kill();
+ }
var inline = this._getInlineForEvent(e);
var is_ref = false;
@@ -2172,6 +2181,9 @@
case 'reply':
inline.reply();
break;
+ case 'draft':
+ inline.triggerDraft();
+ break;
}
}
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
@@ -42,6 +42,8 @@
_undoType: null,
_undoText: null,
+ _draftRequest: null,
+
bindToRow: function(row) {
this._row = row;
@@ -89,11 +91,17 @@
this._isEditing = data.isEditing;
if (this._isEditing) {
- this.edit();
+ // NOTE: The "original" shipped down in the DOM may reflect a draft
+ // which we're currently editing. This flow is a little clumsy, but
+ // reasonable until some future change moves away from "send down
+ // the inline, then immediately click edit".
+ this.edit(this._originalText);
} else {
this.setInvisible(false);
}
+ this._startDrafts();
+
return this;
},
@@ -153,6 +161,7 @@
parent_row.parentNode.insertBefore(row, target_row);
this.setInvisible(true);
+ this._startDrafts();
return this;
},
@@ -213,6 +222,7 @@
parent_row.parentNode.insertBefore(row, target_row);
this.setInvisible(true);
+ this._startDrafts();
return this;
},
@@ -795,7 +805,59 @@
var changeset = this.getChangeset();
var list = changeset.getChangesetList();
return list.getInlineURI();
+ },
+
+ _startDrafts: function() {
+ if (this._draftRequest) {
+ return;
+ }
+
+ var onresponse = JX.bind(this, this._onDraftResponse);
+ var draft = JX.bind(this, this._getDraftState);
+
+ var uri = this._getInlineURI();
+ var request = new JX.PhabricatorShapedRequest(uri, onresponse, draft);
+
+ // The main transaction code uses a 500ms delay on desktop and a
+ // 10s delay on mobile. Perhaps this should be standardized.
+ request.setRateLimit(2000);
+
+ this._draftRequest = request;
+
+ request.start();
+ },
+
+ _onDraftResponse: function() {
+ // For now, do nothing.
+ },
+
+ _getDraftState: function() {
+ if (this.isDeleted()) {
+ return null;
+ }
+
+ if (!this.isEditing()) {
+ return null;
+ }
+
+ var text = this._readText(this._editRow);
+ if (text === null) {
+ return null;
+ }
+
+ return {
+ op: 'draft',
+ id: this.getID(),
+ text: text
+ };
+ },
+
+ triggerDraft: function() {
+ if (this._draftRequest) {
+ this._draftRequest.trigger();
+ }
}
+
}
});
diff --git a/webroot/rsrc/js/core/ShapedRequest.js b/webroot/rsrc/js/core/ShapedRequest.js
--- a/webroot/rsrc/js/core/ShapedRequest.js
+++ b/webroot/rsrc/js/core/ShapedRequest.js
@@ -81,6 +81,10 @@
},
shouldSendRequest : function(last, data) {
+ if (data === null) {
+ return false;
+ }
+
if (last === null) {
return true;
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mon, Mar 31, 9:18 PM (2 d, 13 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7475382
Default Alt Text
D21216.diff (20 KB)
Attached To
Mode
D21216: Save drafts for inline comments currently being edited
Attached
Detach File
Event Timeline
Log In to Comment