mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-26 00:32:42 +01:00
Clean up Diffusion behaviors for inline edit suggestions
Summary: Ref T13513. For now, I'm not supporting inline edit suggestions in Diffusion, although it's likely not difficult to do so in the future. Clean up some of the code so that plain ol' inlines work the same way they did before. Test Plan: - Created, edited, reloaded, submitted inlines in Diffusion: familiar old behavior. - Created, edited, reloaded, submitted inlines with suggestions in Differential: familiar new behavior. Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21278
This commit is contained in:
parent
10f241352d
commit
d2d7e7f5ff
5 changed files with 92 additions and 74 deletions
|
@ -13,7 +13,7 @@ return array(
|
||||||
'core.pkg.js' => '845355f4',
|
'core.pkg.js' => '845355f4',
|
||||||
'dark-console.pkg.js' => '187792c2',
|
'dark-console.pkg.js' => '187792c2',
|
||||||
'differential.pkg.css' => '5c459f92',
|
'differential.pkg.css' => '5c459f92',
|
||||||
'differential.pkg.js' => '256a327a',
|
'differential.pkg.js' => '24616785',
|
||||||
'diffusion.pkg.css' => '42c75c37',
|
'diffusion.pkg.css' => '42c75c37',
|
||||||
'diffusion.pkg.js' => 'a98c0bf7',
|
'diffusion.pkg.js' => 'a98c0bf7',
|
||||||
'maniphest.pkg.css' => '35995d6d',
|
'maniphest.pkg.css' => '35995d6d',
|
||||||
|
@ -381,7 +381,7 @@ return array(
|
||||||
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
|
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
|
||||||
'rsrc/js/application/diff/DiffChangeset.js' => '6e5e03d2',
|
'rsrc/js/application/diff/DiffChangeset.js' => '6e5e03d2',
|
||||||
'rsrc/js/application/diff/DiffChangesetList.js' => 'b51ba93a',
|
'rsrc/js/application/diff/DiffChangesetList.js' => 'b51ba93a',
|
||||||
'rsrc/js/application/diff/DiffInline.js' => '829b88bf',
|
'rsrc/js/application/diff/DiffInline.js' => '008b6a15',
|
||||||
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
|
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
|
||||||
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
|
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
|
||||||
'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
|
'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
|
||||||
|
@ -776,7 +776,7 @@ return array(
|
||||||
'phabricator-dashboard-css' => '5a205b9d',
|
'phabricator-dashboard-css' => '5a205b9d',
|
||||||
'phabricator-diff-changeset' => '6e5e03d2',
|
'phabricator-diff-changeset' => '6e5e03d2',
|
||||||
'phabricator-diff-changeset-list' => 'b51ba93a',
|
'phabricator-diff-changeset-list' => 'b51ba93a',
|
||||||
'phabricator-diff-inline' => '829b88bf',
|
'phabricator-diff-inline' => '008b6a15',
|
||||||
'phabricator-diff-path-view' => '8207abf9',
|
'phabricator-diff-path-view' => '8207abf9',
|
||||||
'phabricator-diff-tree-view' => '5d83623b',
|
'phabricator-diff-tree-view' => '5d83623b',
|
||||||
'phabricator-drag-and-drop-file-upload' => '4370900d',
|
'phabricator-drag-and-drop-file-upload' => '4370900d',
|
||||||
|
@ -917,6 +917,9 @@ return array(
|
||||||
'unhandled-exception-css' => '9ecfc00d',
|
'unhandled-exception-css' => '9ecfc00d',
|
||||||
),
|
),
|
||||||
'requires' => array(
|
'requires' => array(
|
||||||
|
'008b6a15' => array(
|
||||||
|
'javelin-dom',
|
||||||
|
),
|
||||||
'0116d3e8' => array(
|
'0116d3e8' => array(
|
||||||
'javelin-behavior',
|
'javelin-behavior',
|
||||||
'javelin-dom',
|
'javelin-dom',
|
||||||
|
@ -1639,9 +1642,6 @@ return array(
|
||||||
'8207abf9' => array(
|
'8207abf9' => array(
|
||||||
'javelin-dom',
|
'javelin-dom',
|
||||||
),
|
),
|
||||||
'829b88bf' => array(
|
|
||||||
'javelin-dom',
|
|
||||||
),
|
|
||||||
83754533 => array(
|
83754533 => array(
|
||||||
'javelin-install',
|
'javelin-install',
|
||||||
'javelin-util',
|
'javelin-util',
|
||||||
|
|
|
@ -67,4 +67,78 @@ final class DifferentialDiffInlineCommentQuery
|
||||||
return $id_map;
|
return $id_map;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function newInlineContextMap(array $inlines) {
|
||||||
|
$viewer = $this->getViewer();
|
||||||
|
|
||||||
|
$map = array();
|
||||||
|
|
||||||
|
foreach ($inlines as $key => $inline) {
|
||||||
|
$changeset = id(new DifferentialChangesetQuery())
|
||||||
|
->setViewer($viewer)
|
||||||
|
->withIDs(array($inline->getChangesetID()))
|
||||||
|
->needHunks(true)
|
||||||
|
->executeOne();
|
||||||
|
if (!$changeset) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$hunks = $changeset->getHunks();
|
||||||
|
|
||||||
|
$is_simple =
|
||||||
|
(count($hunks) === 1) &&
|
||||||
|
((int)head($hunks)->getOldOffset() <= 1) &&
|
||||||
|
((int)head($hunks)->getNewOffset() <= 1);
|
||||||
|
|
||||||
|
if (!$is_simple) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($inline->getIsNewFile()) {
|
||||||
|
$vector = $changeset->getNewStatePathVector();
|
||||||
|
$filename = last($vector);
|
||||||
|
$corpus = $changeset->makeNewFile();
|
||||||
|
} else {
|
||||||
|
$vector = $changeset->getOldStatePathVector();
|
||||||
|
$filename = last($vector);
|
||||||
|
$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())
|
||||||
|
->setFilename($filename)
|
||||||
|
->setHeadLines($head)
|
||||||
|
->setBodyLines($body)
|
||||||
|
->setTailLines($tail);
|
||||||
|
|
||||||
|
$map[$key] = $context;
|
||||||
|
}
|
||||||
|
|
||||||
|
return $map;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -66,4 +66,8 @@ final class DiffusionDiffInlineCommentQuery
|
||||||
return array();
|
return array();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function newInlineContextMap(array $inlines) {
|
||||||
|
return array();
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -18,6 +18,8 @@ abstract class PhabricatorDiffInlineCommentQuery
|
||||||
$viewer_phid,
|
$viewer_phid,
|
||||||
array $comments);
|
array $comments);
|
||||||
|
|
||||||
|
abstract protected function newInlineContextMap(array $inlines);
|
||||||
|
|
||||||
final public function withFixedStates(array $states) {
|
final public function withFixedStates(array $states) {
|
||||||
$this->fixedStates = $states;
|
$this->fixedStates = $states;
|
||||||
return $this;
|
return $this;
|
||||||
|
@ -265,72 +267,12 @@ abstract class PhabricatorDiffInlineCommentQuery
|
||||||
$need_context[] = $inline;
|
$need_context[] = $inline;
|
||||||
}
|
}
|
||||||
|
|
||||||
foreach ($need_context as $inline) {
|
if ($need_context) {
|
||||||
$changeset = id(new DifferentialChangesetQuery())
|
$context_map = $this->newInlineContextMap($need_context);
|
||||||
->setViewer($viewer)
|
|
||||||
->withIDs(array($inline->getChangesetID()))
|
foreach ($need_context as $key => $inline) {
|
||||||
->needHunks(true)
|
$inline->attachInlineContext(idx($context_map, $key));
|
||||||
->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()) {
|
|
||||||
$vector = $changeset->getNewStatePathVector();
|
|
||||||
$filename = last($vector);
|
|
||||||
$corpus = $changeset->makeNewFile();
|
|
||||||
} else {
|
|
||||||
$vector = $changeset->getOldStatePathVector();
|
|
||||||
$filename = last($vector);
|
|
||||||
$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())
|
|
||||||
->setFilename($filename)
|
|
||||||
->setHeadLines($head)
|
|
||||||
->setBodyLines($body)
|
|
||||||
->setTailLines($tail);
|
|
||||||
|
|
||||||
$inline->attachInlineContext($context);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
@ -338,7 +280,7 @@ abstract class PhabricatorDiffInlineCommentQuery
|
||||||
return $inlines;
|
return $inlines;
|
||||||
}
|
}
|
||||||
|
|
||||||
private function simplifyContext(array $lines, $is_head) {
|
final protected function simplifyContext(array $lines, $is_head) {
|
||||||
// We want to provide the smallest amount of context we can while still
|
// 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
|
// being useful, since the actual code is visible nearby and showing a
|
||||||
// ton of context is silly.
|
// ton of context is silly.
|
||||||
|
|
|
@ -668,8 +668,6 @@ JX.install('DiffInline', {
|
||||||
this._editRow = this._drawRows(rows, null, 'edit');
|
this._editRow = this._drawRows(rows, null, 'edit');
|
||||||
|
|
||||||
this._drawSuggestionState(this._editRow);
|
this._drawSuggestionState(this._editRow);
|
||||||
JX.log(this._originalState);
|
|
||||||
|
|
||||||
this.setHasSuggestion(this._originalState.hasSuggestion);
|
this.setHasSuggestion(this._originalState.hasSuggestion);
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue