diff --git a/resources/celerity/map.php b/resources/celerity/map.php index e57a9bfc0f..434ca6e869 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -12,8 +12,8 @@ return array( 'core.pkg.css' => 'ba768cdb', 'core.pkg.js' => '845355f4', 'dark-console.pkg.js' => '187792c2', - 'differential.pkg.css' => '42a2334f', - 'differential.pkg.js' => 'd0ddfb19', + 'differential.pkg.css' => 'f924dbcf', + 'differential.pkg.js' => '256a327a', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -65,7 +65,7 @@ return array( 'rsrc/css/application/differential/add-comment.css' => '7e5900d9', 'rsrc/css/application/differential/changeset-view.css' => '60c3d405', 'rsrc/css/application/differential/core.css' => '7300a73e', - 'rsrc/css/application/differential/phui-inline-comment.css' => 'd5749acc', + 'rsrc/css/application/differential/phui-inline-comment.css' => '4107254a', 'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d', 'rsrc/css/application/differential/revision-history.css' => '8aa3eac5', 'rsrc/css/application/differential/revision-list.css' => '93d2df7d', @@ -381,7 +381,7 @@ return array( '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/DiffInline.js' => '6fa445ef', + 'rsrc/js/application/diff/DiffInline.js' => '829b88bf', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', 'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b', 'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd', @@ -776,7 +776,7 @@ return array( 'phabricator-dashboard-css' => '5a205b9d', 'phabricator-diff-changeset' => '6e5e03d2', 'phabricator-diff-changeset-list' => 'b51ba93a', - 'phabricator-diff-inline' => '6fa445ef', + 'phabricator-diff-inline' => '829b88bf', 'phabricator-diff-path-view' => '8207abf9', 'phabricator-diff-tree-view' => '5d83623b', 'phabricator-drag-and-drop-file-upload' => '4370900d', @@ -854,7 +854,7 @@ return array( 'phui-icon-view-css' => '4cbc684a', 'phui-image-mask-css' => '62c7f4d2', 'phui-info-view-css' => 'a10a909b', - 'phui-inline-comment-view-css' => 'd5749acc', + 'phui-inline-comment-view-css' => '4107254a', 'phui-invisible-character-view-css' => 'c694c4a4', 'phui-left-right-css' => '68513c34', 'phui-lightbox-css' => '4ebf22da', @@ -1561,9 +1561,6 @@ return array( 'phabricator-diff-path-view', 'phuix-button-view', ), - '6fa445ef' => array( - 'javelin-dom', - ), 70245195 => array( 'javelin-behavior', 'javelin-stratcom', @@ -1642,6 +1639,9 @@ return array( '8207abf9' => array( 'javelin-dom', ), + '829b88bf' => array( + 'javelin-dom', + ), 83754533 => array( 'javelin-install', 'javelin-util', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 114dcc98d0..fa3eaabe7a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3161,6 +3161,7 @@ phutil_register_library_map(array( 'PhabricatorDeveloperConfigOptions' => 'applications/config/option/PhabricatorDeveloperConfigOptions.php', 'PhabricatorDeveloperPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDeveloperPreferencesSettingsPanel.php', 'PhabricatorDiffInlineCommentContentState' => 'infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php', + 'PhabricatorDiffInlineCommentContext' => 'infrastructure/diff/inline/PhabricatorDiffInlineCommentContext.php', 'PhabricatorDiffInlineCommentQuery' => 'infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php', 'PhabricatorDiffPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDiffPreferencesSettingsPanel.php', 'PhabricatorDiffScopeEngine' => 'infrastructure/diff/PhabricatorDiffScopeEngine.php', @@ -3594,6 +3595,7 @@ phutil_register_library_map(array( 'PhabricatorInlineComment' => 'infrastructure/diff/interface/PhabricatorInlineComment.php', 'PhabricatorInlineCommentAdjustmentEngine' => 'infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php', 'PhabricatorInlineCommentContentState' => 'infrastructure/diff/inline/PhabricatorInlineCommentContentState.php', + 'PhabricatorInlineCommentContext' => 'infrastructure/diff/inline/PhabricatorInlineCommentContext.php', 'PhabricatorInlineCommentController' => 'infrastructure/diff/PhabricatorInlineCommentController.php', 'PhabricatorInlineCommentInterface' => 'applications/transactions/interface/PhabricatorInlineCommentInterface.php', 'PhabricatorInlineSummaryView' => 'infrastructure/diff/view/PhabricatorInlineSummaryView.php', @@ -9630,6 +9632,7 @@ phutil_register_library_map(array( 'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorDeveloperPreferencesSettingsPanel' => 'PhabricatorEditEngineSettingsPanel', 'PhabricatorDiffInlineCommentContentState' => 'PhabricatorInlineCommentContentState', + 'PhabricatorDiffInlineCommentContext' => 'PhabricatorInlineCommentContext', 'PhabricatorDiffInlineCommentQuery' => 'PhabricatorApplicationTransactionCommentQuery', 'PhabricatorDiffPreferencesSettingsPanel' => 'PhabricatorEditEngineSettingsPanel', 'PhabricatorDiffScopeEngine' => 'Phobject', @@ -10122,6 +10125,7 @@ phutil_register_library_map(array( ), 'PhabricatorInlineCommentAdjustmentEngine' => 'Phobject', 'PhabricatorInlineCommentContentState' => 'Phobject', + 'PhabricatorInlineCommentContext' => 'Phobject', 'PhabricatorInlineCommentController' => 'PhabricatorController', 'PhabricatorInlineSummaryView' => 'AphrontView', 'PhabricatorInstructionsEditField' => 'PhabricatorEditField', diff --git a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php index 01f173bd9a..cb9d5e20eb 100644 --- a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php +++ b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php @@ -17,6 +17,7 @@ final class PhabricatorAuditTransactionComment protected $attributes = array(); private $replyToComment = self::ATTACHABLE; + private $inlineContext = self::ATTACHABLE; public function getApplicationTransactionObject() { return new PhabricatorAuditTransaction(); @@ -83,12 +84,18 @@ final class PhabricatorAuditTransactionComment return $this; } - public function isEmptyInlineComment() { - return !strlen($this->getContent()); - } - public function newInlineCommentObject() { return PhabricatorAuditInlineComment::newFromModernComment($this); } + public function getInlineContext() { + return $this->assertAttached($this->inlineContext); + } + + public function attachInlineContext( + PhabricatorInlineCommentContext $context = null) { + $this->inlineContext = $context; + return $this; + } + } diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index df1ffa1d05..0e263d013c 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -200,6 +200,7 @@ final class DifferentialChangesetViewController extends DifferentialController { ->withPublishableComments(true) ->withPublishedComments(true) ->needHidden(true) + ->needInlineContext(true) ->execute(); $inlines = mpull($inlines, 'newInlineCommentObject'); diff --git a/src/applications/differential/editor/DifferentialRevisionEditEngine.php b/src/applications/differential/editor/DifferentialRevisionEditEngine.php index c5b8311630..cc90e36a61 100644 --- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php +++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php @@ -328,6 +328,13 @@ final class DifferentialRevisionEditEngine $content = array(); if ($inlines) { + // Reload inlines to get inline context. + $inlines = id(new DifferentialDiffInlineCommentQuery()) + ->setViewer($viewer) + ->withIDs(mpull($inlines, 'getID')) + ->needInlineContext(true) + ->execute(); + $inline_preview = id(new PHUIDiffInlineCommentPreviewListView()) ->setViewer($viewer) ->setInlineComments($inlines); diff --git a/src/applications/differential/storage/DifferentialTransactionComment.php b/src/applications/differential/storage/DifferentialTransactionComment.php index f5cb772d48..491d89a968 100644 --- a/src/applications/differential/storage/DifferentialTransactionComment.php +++ b/src/applications/differential/storage/DifferentialTransactionComment.php @@ -18,6 +18,7 @@ final class DifferentialTransactionComment private $replyToComment = self::ATTACHABLE; private $isHidden = self::ATTACHABLE; private $changeset = self::ATTACHABLE; + private $inlineContext = self::ATTACHABLE; public function getApplicationTransactionObject() { return new DifferentialTransaction(); @@ -129,12 +130,18 @@ final class DifferentialTransactionComment return $this; } - public function isEmptyInlineComment() { - return !strlen($this->getContent()); - } - public function newInlineCommentObject() { return DifferentialInlineComment::newFromModernComment($this); } + public function getInlineContext() { + return $this->assertAttached($this->inlineContext); + } + + public function attachInlineContext( + PhabricatorInlineCommentContext $context = null) { + $this->inlineContext = $context; + return $this; + } + } diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index bf8aa8fae5..cab63eb998 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -373,6 +373,9 @@ final class DifferentialChangesetListView extends AphrontView { 'Add new inline comment on selected source text.' => pht('Add new inline comment on selected source text.'), + + 'Suggest Edit' => pht('Suggest Edit'), + 'Discard Edit' => pht('Discard Edit'), ), )); diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php index 83838e8385..e1b8dc7264 100644 --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -240,7 +240,7 @@ abstract class PhabricatorInlineCommentController $view = $this->buildScaffoldForView($edit_dialog); - return $this->newInlineResponse($inline, $view); + return $this->newInlineResponse($inline, $view, true); case 'cancel': $inline = $this->loadCommentByIDForEdit($this->getCommentID()); @@ -325,6 +325,9 @@ abstract class PhabricatorInlineCommentController $this->saveComment($inline); + // Reload the inline to attach context. + $inline = $this->loadCommentByIDForEdit($inline->getID()); + $edit_dialog = $this->buildEditDialog($inline); if ($this->getOperation() == 'reply') { @@ -335,7 +338,7 @@ abstract class PhabricatorInlineCommentController $view = $this->buildScaffoldForView($edit_dialog); - return $this->newInlineResponse($inline, $view); + return $this->newInlineResponse($inline, $view, true); } } @@ -431,7 +434,7 @@ abstract class PhabricatorInlineCommentController $view = $this->buildScaffoldForView($view); - return $this->newInlineResponse($inline, $view); + return $this->newInlineResponse($inline, $view, false); } private function buildScaffoldForView(PHUIDiffInlineCommentView $view) { @@ -446,11 +449,29 @@ abstract class PhabricatorInlineCommentController private function newInlineResponse( PhabricatorInlineComment $inline, - $view) { + $view, + $is_edit) { + + if ($inline->getReplyToCommentPHID()) { + $can_suggest = false; + } else { + $can_suggest = (bool)$inline->getInlineContext(); + } + + if ($is_edit) { + $viewer = $this->getViewer(); + $content_state = $inline->getContentStateForEdit($viewer); + } else { + $content_state = $inline->getContentState(); + } + + $state_map = $content_state->newStorageMap(); $response = array( 'inline' => array( 'id' => $inline->getID(), + 'contentState' => $state_map, + 'canSuggestEdit' => $can_suggest, ), 'view' => hsprintf('%s', $view), ); @@ -477,7 +498,8 @@ abstract class PhabricatorInlineCommentController $viewer = $this->getViewer(); $query = $this->newInlineCommentQuery() - ->withIDs(array($id)); + ->withIDs(array($id)) + ->needInlineContext(true); $inline = $this->loadCommentByQuery($query); diff --git a/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php b/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php index 81a520b0cf..0f379acc3e 100644 --- a/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php +++ b/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php @@ -12,7 +12,7 @@ final class PhabricatorDiffInlineCommentContentState } if ($this->getContentHasSuggestion()) { - if (strlen($this->getSuggestionText())) { + if (strlen($this->getContentSuggestionText())) { return false; } } diff --git a/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContext.php b/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContext.php new file mode 100644 index 0000000000..e46d934b5a --- /dev/null +++ b/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContext.php @@ -0,0 +1,37 @@ +headLines = $head_lines; + return $this; + } + + public function getHeadLines() { + return $this->headLines; + } + + public function setBodyLines(array $body_lines) { + $this->bodyLines = $body_lines; + return $this; + } + + public function getBodyLines() { + return $this->bodyLines; + } + + public function setTailLines(array $tail_lines) { + $this->tailLines = $tail_lines; + return $this; + } + + public function getTailLines() { + return $this->tailLines; + } + +} diff --git a/src/infrastructure/diff/inline/PhabricatorInlineCommentContext.php b/src/infrastructure/diff/inline/PhabricatorInlineCommentContext.php new file mode 100644 index 0000000000..a292dc72ee --- /dev/null +++ b/src/infrastructure/diff/inline/PhabricatorInlineCommentContext.php @@ -0,0 +1,4 @@ +getStorageObject()->getInlineContext(); + } + /* -( PhabricatorMarkupInterface Implementation )-------------------------- */ diff --git a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php index 4e846a0495..b5f39b1d2f 100644 --- a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php +++ b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php @@ -9,6 +9,7 @@ abstract class PhabricatorDiffInlineCommentQuery private $publishableComments; private $needHidden; private $needAppliedDrafts; + private $needInlineContext; abstract protected function buildInlineCommentWhereClauseParts( AphrontDatabaseConnection $conn); @@ -42,6 +43,11 @@ abstract class PhabricatorDiffInlineCommentQuery return $this; } + final public function needInlineContext($need_context) { + $this->needInlineContext = $need_context; + return $this; + } + final public function needAppliedDrafts($need_applied) { $this->needAppliedDrafts = $need_applied; return $this; @@ -173,26 +179,6 @@ abstract class PhabricatorDiffInlineCommentQuery return $inlines; } - if ($this->needHidden) { - $viewer_phid = $viewer->getPHID(); - - if ($viewer_phid) { - $hidden = $this->loadHiddenCommentIDs( - $viewer_phid, - $inlines); - } else { - $hidden = array(); - } - - foreach ($inlines as $inline) { - $inline->attachIsHidden(isset($hidden[$inline->getID()])); - } - } - - if (!$inlines) { - return $inlines; - } - $need_drafts = $this->needAppliedDrafts; $drop_void = $this->publishableComments; $convert_objects = ($need_drafts || $drop_void); @@ -247,4 +233,133 @@ abstract class PhabricatorDiffInlineCommentQuery return $inlines; } + protected function didFilterPage(array $inlines) { + $viewer = $this->getViewer(); + + if ($this->needHidden) { + $viewer_phid = $viewer->getPHID(); + + if ($viewer_phid) { + $hidden = $this->loadHiddenCommentIDs( + $viewer_phid, + $inlines); + } else { + $hidden = array(); + } + + foreach ($inlines as $inline) { + $inline->attachIsHidden(isset($hidden[$inline->getID()])); + } + } + + if ($this->needInlineContext) { + $need_context = array(); + foreach ($inlines as $inline) { + $object = $inline->newInlineCommentObject(); + + if ($object->getDocumentEngineKey() !== null) { + $inline->attachInlineContext(null); + continue; + } + + $need_context[] = $inline; + } + + foreach ($need_context as $inline) { + $changeset = id(new DifferentialChangesetQuery()) + ->setViewer($viewer) + ->withIDs(array($inline->getChangesetID())) + ->needHunks(true) + ->executeOne(); + if (!$changeset) { + $inline->attachInlineContext(null); + continue; + } + + $hunks = $changeset->getHunks(); + + $is_simple = + (count($hunks) === 1) && + ((int)head($hunks)->getOldOffset() <= 1) && + ((int)head($hunks)->getNewOffset() <= 1); + + if (!$is_simple) { + $inline->attachInlineContext(null); + continue; + } + + if ($inline->getIsNewFile()) { + $corpus = $changeset->makeNewFile(); + } else { + $corpus = $changeset->makeOldFile(); + } + + $corpus = phutil_split_lines($corpus); + + // Adjust the line number into a 0-based offset. + $offset = $inline->getLineNumber(); + $offset = $offset - 1; + + // Adjust the inclusive range length into a row count. + $length = $inline->getLineLength(); + $length = $length + 1; + + $head_min = max(0, $offset - 3); + $head_max = $offset; + $head_len = $head_max - $head_min; + + if ($head_len) { + $head = array_slice($corpus, $head_min, $head_len, true); + $head = $this->simplifyContext($head, true); + } else { + $head = array(); + } + + $body = array_slice($corpus, $offset, $length, true); + + $tail = array_slice($corpus, $offset + $length, 3, true); + $tail = $this->simplifyContext($tail, false); + + $context = id(new PhabricatorDiffInlineCommentContext()) + ->setHeadLines($head) + ->setBodyLines($body) + ->setTailLines($tail); + + $inline->attachInlineContext($context); + } + + } + + return $inlines; + } + + private function simplifyContext(array $lines, $is_head) { + // We want to provide the smallest amount of context we can while still + // being useful, since the actual code is visible nearby and showing a + // ton of context is silly. + + // Examine each line until we find one that looks "useful" (not just + // whitespace or a single bracket). Once we find a useful piece of context + // to anchor the text, discard the rest of the lines beyond it. + + if ($is_head) { + $lines = array_reverse($lines, true); + } + + $saw_context = false; + foreach ($lines as $key => $line) { + if ($saw_context) { + unset($lines[$key]); + continue; + } + + $saw_context = (strlen(trim($line)) > 3); + } + + if ($is_head) { + $lines = array_reverse($lines, true); + } + + return $lines; + } } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php index db2952faa8..b2a49f0624 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php @@ -427,6 +427,15 @@ final class PHUIDiffInlineCommentDetailView $metadata['menuItems'] = $menu_items; + $suggestion_content = $this->newSuggestionView($inline); + + $inline_content = phutil_tag( + 'div', + array( + 'class' => 'phabricator-remarkup', + ), + $content); + $markup = javelin_tag( 'div', array( @@ -445,9 +454,15 @@ final class PHUIDiffInlineCommentDetailView $group_left, $group_right, )), - phutil_tag_div( - 'differential-inline-comment-content', - phutil_tag_div('phabricator-remarkup', $content)), + phutil_tag( + 'div', + array( + 'class' => 'differential-inline-comment-content', + ), + array( + $suggestion_content, + $inline_content, + )), )); $summary = phutil_tag( @@ -491,4 +506,57 @@ final class PHUIDiffInlineCommentDetailView return true; } + private function newSuggestionView(PhabricatorInlineComment $inline) { + $content_state = $inline->getContentState(); + if (!$content_state->getContentHasSuggestion()) { + return null; + } + + $context = $inline->getInlineContext(); + if (!$context) { + return null; + } + + $head_lines = $context->getHeadLines(); + $head_lines = implode('', $head_lines); + + $tail_lines = $context->getTailLines(); + $tail_lines = implode('', $tail_lines); + + $old_lines = $context->getBodyLines(); + $old_lines = implode('', $old_lines); + $old_lines = $head_lines.$old_lines.$tail_lines; + if (strlen($old_lines) && !preg_match('/\n\z/', $old_lines)) { + $old_lines .= "\n"; + } + + $new_lines = $content_state->getContentSuggestionText(); + $new_lines = $head_lines.$new_lines.$tail_lines; + if (strlen($new_lines) && !preg_match('/\n\z/', $new_lines)) { + $new_lines .= "\n"; + } + + if ($old_lines === $new_lines) { + return null; + } + + + $raw_diff = id(new PhabricatorDifferenceEngine()) + ->generateRawDiffFromFileContent($old_lines, $new_lines); + + $raw_diff = phutil_split_lines($raw_diff); + $raw_diff = array_slice($raw_diff, 3); + $raw_diff = implode('', $raw_diff); + + $view = phutil_tag( + 'div', + array( + 'class' => 'inline-suggestion-view PhabricatorMonospaced', + ), + $raw_diff); + + return $view; + } + + } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php index ce43653119..5c29c1666c 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php @@ -46,17 +46,23 @@ final class PHUIDiffInlineCommentEditView ), $this->title); + $corpus_view = $this->newCorpusView(); + $body = phutil_tag( 'div', array( 'class' => 'differential-inline-comment-edit-body', ), - $this->newTextarea()); + array( + $corpus_view, + $this->newTextarea(), + )); - $edit = phutil_tag( + $edit = javelin_tag( 'div', array( 'class' => 'differential-inline-comment-edit-buttons grouped', + 'sigil' => 'inline-edit-buttons', ), array( $buttons, @@ -91,4 +97,121 @@ final class PHUIDiffInlineCommentEditView ->setDisableFullScreen(true); } + private function newCorpusView() { + $viewer = $this->getViewer(); + $inline = $this->getInlineComment(); + + $context = $inline->getInlineContext(); + if ($context === null) { + return null; + } + + $head = $context->getHeadLines(); + $head = $this->newContextView($head); + + $state = $inline->getContentStateForEdit($viewer); + + $main = $state->getContentSuggestionText(); + $main_count = count(phutil_split_lines($main)); + + $default = $context->getBodyLines(); + $default = implode('', $default); + + // Browsers ignore one leading newline in text areas. Add one so that + // any actual leading newlines in the content are preserved. + $main = "\n".$main; + + $textarea = javelin_tag( + 'textarea', + array( + 'class' => 'inline-suggestion-input PhabricatorMonospaced', + 'rows' => max(3, $main_count + 1), + 'sigil' => 'inline-content-suggestion', + 'meta' => array( + 'defaultText' => $default, + ), + ), + $main); + + $main = phutil_tag( + 'tr', + array( + 'class' => 'inline-suggestion-input-row', + ), + array( + phutil_tag( + 'td', + array( + 'class' => 'inline-suggestion-line-cell', + ), + null), + phutil_tag( + 'td', + array( + 'class' => 'inline-suggestion-input-cell', + ), + $textarea), + )); + + $tail = $context->getTailLines(); + $tail = $this->newContextView($tail); + + $body = phutil_tag( + 'tbody', + array(), + array( + $head, + $main, + $tail, + )); + + $table = phutil_tag( + 'table', + array( + 'class' => 'inline-suggestion-table', + ), + $body); + + $container = phutil_tag( + 'div', + array( + 'class' => 'inline-suggestion', + ), + $table); + + return $container; + } + + private function newContextView(array $lines) { + if (!$lines) { + return array(); + } + + $rows = array(); + foreach ($lines as $index => $line) { + $line_cell = phutil_tag( + 'td', + array( + 'class' => 'inline-suggestion-line-cell PhabricatorMonospaced', + ), + $index + 1); + + $text_cell = phutil_tag( + 'td', + array( + 'class' => 'inline-suggestion-text-cell PhabricatorMonospaced', + ), + $line); + + $cells = array( + $line_cell, + $text_cell, + ); + + $rows[] = phutil_tag('tr', array(), $cells); + } + + return $rows; + } + } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php index 97fc9ee6e5..11be8d0ec1 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php @@ -93,7 +93,7 @@ abstract class PHUIDiffInlineCommentView extends AphrontView { 'startOffset' => $inline->getStartOffset(), 'endOffset' => $inline->getEndOffset(), 'on_right' => $this->getIsOnRight(), - 'contentState' => $inline->getContentState(), + 'contentState' => $inline->getContentState()->newStorageMap(), ); } diff --git a/webroot/rsrc/css/application/differential/phui-inline-comment.css b/webroot/rsrc/css/application/differential/phui-inline-comment.css index ce5b87effd..9fefc65353 100644 --- a/webroot/rsrc/css/application/differential/phui-inline-comment.css +++ b/webroot/rsrc/css/application/differential/phui-inline-comment.css @@ -436,3 +436,60 @@ background: {$lightyellow}; border-color: {$yellow}; } + +.inline-suggestion { + display: none; + margin: 0 -8px; +} + +.has-suggestion .inline-suggestion { + display: block; +} + +.differential-inline-comment-edit-buttons button.inline-button-left { + float: left; + margin: 0 6px 0 0; +} + +.inline-suggestion-table { + table-layout: fixed; + width: 100%; + margin-bottom: 8px; + white-space: pre-wrap; + background: {$greybackground}; + border-width: 1px 0; + border-style: solid; + border-color: {$lightgreyborder}; +} + +textarea.inline-suggestion-input { + width: 100%; + height: auto; + max-width: 100%; +} + +.inline-suggestion-line-cell { + text-align: right; + background: {$darkgreybackground}; + width: 36px; + color: {$greytext}; + border-right: 1px solid {$lightgreyborder}; +} + +.inline-suggestion-input-cell { + padding: 8px; +} + +.inline-suggestion-text-cell { + padding: 0 8px; +} + +.inline-suggestion-view { + padding: 8px 12px; + white-space: pre-wrap; + background: {$greybackground}; + margin: 0 -12px 8px; + border-width: 1px 0; + border-style: solid; + border-color: {$lightgreyborder}; +} diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js index 49f72101f4..e03138abfa 100644 --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -51,6 +51,7 @@ JX.install('DiffInline', { _startOffset: null, _endOffset: null, _isSelected: false, + _canSuggestEdit: false, bindToRow: function(row) { this._row = row; @@ -76,7 +77,6 @@ JX.install('DiffInline', { this._number = parseInt(data.number, 10); this._length = parseInt(data.length, 10); - this._originalState = data.contentState; this._isNewFile = data.isNewFile; this._replyToCommentPHID = data.replyToCommentPHID; @@ -602,6 +602,8 @@ JX.install('DiffInline', { _readInlineState: function(state) { this._id = state.id; + this._originalState = state.contentState; + this._canSuggestEdit = state.canSuggestEdit; }, _ondeleteresponse: function() { @@ -664,6 +666,11 @@ JX.install('DiffInline', { _drawEditRows: function(rows) { this.setEditing(true); this._editRow = this._drawRows(rows, null, 'edit'); + + this._drawSuggestionState(this._editRow); + JX.log(this._originalState); + + this.setHasSuggestion(this._originalState.hasSuggestion); }, _drawRows: function(rows, cursor, type) { @@ -719,6 +726,91 @@ JX.install('DiffInline', { return result_row; }, + _drawSuggestionState: function(row) { + if (this._canSuggestEdit) { + var button = this._getSuggestionButton(); + var node = button.getNode(); + + // As a side effect of form submission, the button may become + // visually disabled. Re-enable it. This is a bit hacky. + JX.DOM.alterClass(node, 'disabled', false); + node.disabled = false; + + var container = JX.DOM.find(row, 'div', 'inline-edit-buttons'); + container.appendChild(node); + } + }, + + _getSuggestionButton: function() { + if (!this._suggestionButton) { + var button = new JX.PHUIXButtonView() + .setIcon('fa-pencil-square-o') + .setColor('grey'); + + var node = button.getNode(); + JX.DOM.alterClass(node, 'inline-button-left', true); + + var onclick = JX.bind(this, this._onSuggestEdit); + JX.DOM.listen(node, 'click', null, onclick); + + this._suggestionButton = button; + } + + return this._suggestionButton; + }, + + _onSuggestEdit: function(e) { + e.kill(); + + this.setHasSuggestion(!this.getHasSuggestion()); + + // The first time the user actually clicks the button and enables + // suggestions for a given editor state, fill the input with the + // underlying text if there isn't any text yet. + if (this.getHasSuggestion()) { + if (this._editRow) { + var node = this._getSuggestionNode(this._editRow); + if (node) { + if (!node.value.length) { + var data = JX.Stratcom.getData(node); + if (!data.hasSetDefault) { + data.hasSetDefault = true; + node.value = data.defaultText; + node.rows = Math.max(3, node.value.split('\n').length); + } + } + } + } + } + + // Save the "hasSuggestion" part of the content state. + this.triggerDraft(); + }, + + setHasSuggestion: function(has_suggestion) { + this._hasSuggestion = has_suggestion; + + var button = this._getSuggestionButton(); + var pht = this.getChangeset().getChangesetList().getTranslations(); + if (has_suggestion) { + button + .setIcon('fa-times') + .setText(pht('Discard Edit')); + } else { + button + .setIcon('fa-plus') + .setText(pht('Suggest Edit')); + } + + if (this._editRow) { + JX.DOM.alterClass(this._editRow, 'has-suggestion', has_suggestion); + } + }, + + getHasSuggestion: function() { + return this._hasSuggestion; + }, + save: function() { var handler = JX.bind(this, this._onsubmitresponse); @@ -825,16 +917,24 @@ JX.install('DiffInline', { // Ignore. } - try { - node = JX.DOM.find(row, 'textarea', 'inline-content-suggestion'); + node = this._getSuggestionNode(row); + if (node) { state.suggestionText = node.value; - } catch (ex) { - // Ignore. } + state.hasSuggestion = this.getHasSuggestion(); + return state; }, + _getSuggestionNode: function(row) { + try { + return JX.DOM.find(row, 'textarea', 'inline-content-suggestion'); + } catch (ex) { + return null; + } + }, + _onsubmitresponse: function(response) { if (this._editRow) { JX.DOM.remove(this._editRow); @@ -1063,7 +1163,7 @@ JX.install('DiffInline', { return { text: '', suggestionText: '', - hasSuggestion: true + hasSuggestion: false }; }, @@ -1073,6 +1173,7 @@ JX.install('DiffInline', { _isSameContentState: function(u, v) { return ( + ((u === null) === (v === null)) && (u.text === v.text) && (u.suggestionText === v.suggestionText) && (u.hasSuggestion === v.hasSuggestion));