2011-01-25 00:52:35 +01:00
|
|
|
<?php
|
|
|
|
|
2012-03-10 00:46:25 +01:00
|
|
|
final class DifferentialChangesetViewController extends DifferentialController {
|
2011-01-25 00:52:35 +01:00
|
|
|
|
2013-09-27 03:45:04 +02:00
|
|
|
public function shouldAllowPublic() {
|
|
|
|
return true;
|
2011-10-24 21:27:16 +02:00
|
|
|
}
|
|
|
|
|
2015-04-10 01:15:13 +02:00
|
|
|
public function handleRequest(AphrontRequest $request) {
|
Track a "Done" state on inline comments
Summary:
Ref T1460. This just barely works, but throwing it up in case any of it sounds mechanically crazy before we build integrations/UI/etc.
Specifically, these are the behaviors:
- You can mark your own draft comments as "done" before you submit them. The intent is to let reviewers mark their stuff advisory/minor/not-important before they submit it, to hint to authors that they don't expect the feedback to necessarily be addressed (maybe it's a joke, maybe it's just discussion, maybe it's "consider..").
- You can mark others' published comments as "done" if you're the revision/commit author. The intent is to keep this lightweight by not requiring an audit trail of who marked what done when. If anyone could mark anything done, we'd have to have some way to show who marked stuff.
- When you mark stuff done (or unmark it), it goes into a "draft" state, where you see the change but others don't see it yet. The intent is twofold:
- Be consistent with how inlines work.
- Allow us to publish a "epriestley updated this revision + epriestley marked 15 inlines as done" story later if we want. This seems more useful than publishing 15 "epriestley marked one thing as done" stories.
- The actual bit where done-ness publishes isn't implemented.
- UI is bare bones.
- No integration with the rest of the UI yet.
Test Plan: Clicked some checkboxes.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: paulshen, chasemp, epriestley
Maniphest Tasks: T1460
Differential Revision: https://secure.phabricator.com/D12033
2015-03-10 02:41:47 +01:00
|
|
|
$viewer = $this->getViewer();
|
2011-02-01 05:38:13 +01:00
|
|
|
|
Move "Rendering References" to the DifferentialChangesetParser level
Summary:
Separates changeset IDs from rendering. Now each changeset has a "rendering
reference" which is basically a description of what the ajax endpoint should
render. For Differential, it's in the form "id/vs". For Diffusion,
"branch/path;commit".
I believe this fixes pretty much all of the bugs related to "show more" breaking
in various obscure ways, although I never got a great repro for T153.
Test Plan:
Clicked "show more" in diffusion change and commit views and differential diff,
diff-of-diff, standalone-diff, standalone-diff-of-diff views. Verified refs and
'whitespace' were always sent correctly.
Made inline comments on diffs and diffs-of-diffs. Used "Reply".
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 274
2011-05-12 06:46:29 +02:00
|
|
|
$rendering_reference = $request->getStr('ref');
|
|
|
|
$parts = explode('/', $rendering_reference);
|
|
|
|
if (count($parts) == 2) {
|
|
|
|
list($id, $vs) = $parts;
|
|
|
|
} else {
|
|
|
|
$id = $parts[0];
|
|
|
|
$vs = 0;
|
|
|
|
}
|
2011-02-04 00:41:58 +01:00
|
|
|
|
Move "Rendering References" to the DifferentialChangesetParser level
Summary:
Separates changeset IDs from rendering. Now each changeset has a "rendering
reference" which is basically a description of what the ajax endpoint should
render. For Differential, it's in the form "id/vs". For Diffusion,
"branch/path;commit".
I believe this fixes pretty much all of the bugs related to "show more" breaking
in various obscure ways, although I never got a great repro for T153.
Test Plan:
Clicked "show more" in diffusion change and commit views and differential diff,
diff-of-diff, standalone-diff, standalone-diff-of-diff views. Verified refs and
'whitespace' were always sent correctly.
Made inline comments on diffs and diffs-of-diffs. Used "Reply".
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 274
2011-05-12 06:46:29 +02:00
|
|
|
$id = (int)$id;
|
|
|
|
$vs = (int)$vs;
|
2011-02-04 00:41:58 +01:00
|
|
|
|
2014-05-25 17:59:31 +02:00
|
|
|
$load_ids = array($id);
|
|
|
|
if ($vs && ($vs != -1)) {
|
|
|
|
$load_ids[] = $vs;
|
2011-01-25 00:52:35 +01:00
|
|
|
}
|
|
|
|
|
2014-05-25 17:59:31 +02:00
|
|
|
$changesets = id(new DifferentialChangesetQuery())
|
Track a "Done" state on inline comments
Summary:
Ref T1460. This just barely works, but throwing it up in case any of it sounds mechanically crazy before we build integrations/UI/etc.
Specifically, these are the behaviors:
- You can mark your own draft comments as "done" before you submit them. The intent is to let reviewers mark their stuff advisory/minor/not-important before they submit it, to hint to authors that they don't expect the feedback to necessarily be addressed (maybe it's a joke, maybe it's just discussion, maybe it's "consider..").
- You can mark others' published comments as "done" if you're the revision/commit author. The intent is to keep this lightweight by not requiring an audit trail of who marked what done when. If anyone could mark anything done, we'd have to have some way to show who marked stuff.
- When you mark stuff done (or unmark it), it goes into a "draft" state, where you see the change but others don't see it yet. The intent is twofold:
- Be consistent with how inlines work.
- Allow us to publish a "epriestley updated this revision + epriestley marked 15 inlines as done" story later if we want. This seems more useful than publishing 15 "epriestley marked one thing as done" stories.
- The actual bit where done-ness publishes isn't implemented.
- UI is bare bones.
- No integration with the rest of the UI yet.
Test Plan: Clicked some checkboxes.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: paulshen, chasemp, epriestley
Maniphest Tasks: T1460
Differential Revision: https://secure.phabricator.com/D12033
2015-03-10 02:41:47 +01:00
|
|
|
->setViewer($viewer)
|
2014-05-25 17:59:31 +02:00
|
|
|
->withIDs($load_ids)
|
|
|
|
->needHunks(true)
|
|
|
|
->execute();
|
|
|
|
$changesets = mpull($changesets, null, 'getID');
|
|
|
|
|
|
|
|
$changeset = idx($changesets, $id);
|
|
|
|
if (!$changeset) {
|
2013-09-27 03:45:04 +02:00
|
|
|
return new Aphront404Response();
|
|
|
|
}
|
|
|
|
|
2014-05-25 17:59:31 +02:00
|
|
|
$vs_changeset = null;
|
|
|
|
if ($vs && ($vs != -1)) {
|
|
|
|
$vs_changeset = idx($changesets, $vs);
|
|
|
|
if (!$vs_changeset) {
|
|
|
|
return new Aphront404Response();
|
|
|
|
}
|
|
|
|
}
|
2013-09-27 03:45:04 +02:00
|
|
|
|
2011-05-06 21:58:53 +02:00
|
|
|
$view = $request->getStr('view');
|
|
|
|
if ($view) {
|
2012-02-06 20:58:21 +01:00
|
|
|
$phid = idx($changeset->getMetadata(), "$view:binary-phid");
|
|
|
|
if ($phid) {
|
|
|
|
return id(new AphrontRedirectResponse())->setURI("/file/info/$phid/");
|
|
|
|
}
|
|
|
|
switch ($view) {
|
|
|
|
case 'new':
|
2012-02-15 02:00:20 +01:00
|
|
|
return $this->buildRawFileResponse($changeset, $is_new = true);
|
2012-02-06 20:58:21 +01:00
|
|
|
case 'old':
|
2014-05-25 17:59:31 +02:00
|
|
|
if ($vs_changeset) {
|
|
|
|
return $this->buildRawFileResponse($vs_changeset, $is_new = true);
|
2012-08-08 03:53:22 +02:00
|
|
|
}
|
2012-02-15 02:00:20 +01:00
|
|
|
return $this->buildRawFileResponse($changeset, $is_new = false);
|
2012-02-06 20:58:21 +01:00
|
|
|
default:
|
|
|
|
return new Aphront400Response();
|
2011-05-06 21:58:53 +02:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2011-02-04 00:41:58 +01:00
|
|
|
if (!$vs) {
|
|
|
|
$right = $changeset;
|
|
|
|
$left = null;
|
|
|
|
|
|
|
|
$right_source = $right->getID();
|
|
|
|
$right_new = true;
|
|
|
|
$left_source = $right->getID();
|
|
|
|
$left_new = false;
|
2011-05-05 16:08:10 +02:00
|
|
|
|
|
|
|
$render_cache_key = $right->getID();
|
2011-02-04 00:41:58 +01:00
|
|
|
} else if ($vs == -1) {
|
|
|
|
$right = null;
|
|
|
|
$left = $changeset;
|
|
|
|
|
|
|
|
$right_source = $left->getID();
|
|
|
|
$right_new = false;
|
|
|
|
$left_source = $left->getID();
|
|
|
|
$left_new = true;
|
2011-05-05 16:08:10 +02:00
|
|
|
|
|
|
|
$render_cache_key = null;
|
2011-02-04 00:41:58 +01:00
|
|
|
} else {
|
|
|
|
$right = $changeset;
|
|
|
|
$left = $vs_changeset;
|
|
|
|
|
|
|
|
$right_source = $right->getID();
|
|
|
|
$right_new = true;
|
|
|
|
$left_source = $left->getID();
|
|
|
|
$left_new = true;
|
2011-05-05 16:08:10 +02:00
|
|
|
|
|
|
|
$render_cache_key = null;
|
2011-02-04 00:41:58 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
if ($left) {
|
|
|
|
$left_data = $left->makeNewFile();
|
|
|
|
if ($right) {
|
|
|
|
$right_data = $right->makeNewFile();
|
|
|
|
} else {
|
|
|
|
$right_data = $left->makeOldFile();
|
|
|
|
}
|
|
|
|
|
2011-07-17 20:06:02 +02:00
|
|
|
$engine = new PhabricatorDifferenceEngine();
|
|
|
|
$synthetic = $engine->generateChangesetFromFileContent(
|
|
|
|
$left_data,
|
|
|
|
$right_data);
|
2011-02-04 00:41:58 +01:00
|
|
|
|
2012-06-15 09:53:26 +02:00
|
|
|
$choice = clone nonempty($left, $right);
|
2011-07-17 20:06:02 +02:00
|
|
|
$choice->attachHunks($synthetic->getHunks());
|
2011-02-04 00:41:58 +01:00
|
|
|
|
|
|
|
$changeset = $choice;
|
|
|
|
}
|
2011-01-25 00:52:35 +01:00
|
|
|
|
Show coverage information in Differential
Summary:
Render coverage information in the right gutter, if available.
We could render some kind of summary report deal too but this seems like a good
start.
Test Plan:
- Looked at diffs with coverage.
- Looked at diffs without coverage.
- Used inline comments, diff-of-diff, "show more", "show entire file", "show
generated file", "undo". Nothing seemed disrupted by the addition of a 5th
column.
Reviewers: btrahan, tuomaspelkonen, jungejason
Reviewed By: btrahan
CC: zeeg, aran, epriestley
Maniphest Tasks: T140
Differential Revision: https://secure.phabricator.com/D1527
2012-01-31 21:07:47 +01:00
|
|
|
$coverage = null;
|
2012-03-02 02:57:19 +01:00
|
|
|
if ($right && $right->getDiffID()) {
|
Show coverage information in Differential
Summary:
Render coverage information in the right gutter, if available.
We could render some kind of summary report deal too but this seems like a good
start.
Test Plan:
- Looked at diffs with coverage.
- Looked at diffs without coverage.
- Used inline comments, diff-of-diff, "show more", "show entire file", "show
generated file", "undo". Nothing seemed disrupted by the addition of a 5th
column.
Reviewers: btrahan, tuomaspelkonen, jungejason
Reviewed By: btrahan
CC: zeeg, aran, epriestley
Maniphest Tasks: T140
Differential Revision: https://secure.phabricator.com/D1527
2012-01-31 21:07:47 +01:00
|
|
|
$unit = id(new DifferentialDiffProperty())->loadOneWhere(
|
|
|
|
'diffID = %d AND name = %s',
|
|
|
|
$right->getDiffID(),
|
|
|
|
'arc:unit');
|
|
|
|
|
|
|
|
if ($unit) {
|
|
|
|
$coverage = array();
|
|
|
|
foreach ($unit->getData() as $result) {
|
|
|
|
$result_coverage = idx($result, 'coverage');
|
|
|
|
if (!$result_coverage) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
$file_coverage = idx($result_coverage, $right->getFileName());
|
|
|
|
if (!$file_coverage) {
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
$coverage[] = $file_coverage;
|
|
|
|
}
|
|
|
|
|
|
|
|
$coverage = ArcanistUnitTestResult::mergeCoverage($coverage);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2011-07-17 20:06:02 +02:00
|
|
|
$spec = $request->getStr('range');
|
|
|
|
list($range_s, $range_e, $mask) =
|
|
|
|
DifferentialChangesetParser::parseRangeSpecification($spec);
|
2011-02-01 05:38:13 +01:00
|
|
|
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
$parser = id(new DifferentialChangesetParser())
|
|
|
|
->setCoverage($coverage)
|
|
|
|
->setChangeset($changeset)
|
|
|
|
->setRenderingReference($rendering_reference)
|
|
|
|
->setRenderCacheKey($render_cache_key)
|
|
|
|
->setRightSideCommentMapping($right_source, $right_new)
|
|
|
|
->setLeftSideCommentMapping($left_source, $left_new);
|
2011-02-02 22:48:52 +01:00
|
|
|
|
Make "Show Context" persist rendering, whitespace, encoding, etc
Summary:
Ref T2009. Currently, we do not persist view parameters when making context rendering requests.
The big one is the renderer (1up vs 2up). This makes context on unified diffs come in with too many columns.
However, it impacts other parameters too. For example, at HEAD, if you change highlighting to "rainbow" and then load more context, the context uses the original highlighter instead of the rainbow highlighter.
This moves context loads into ChangesetViewManager, which maintains view parameters and can provide them correctly.
- This removes "ref"; it is no longer required, as the ChangesetViewManager tracks it.
- This removes URI management from `behavior-show-more`; it is no longer required, since the ChangesetViewManager knows how to render.
- This removes "whitespace" since this is handled properly by the view manager.
Test Plan:
- Used "Show Top" / "Show All" / "Show Bottom" in 1-up and 2-up views.
- Changed file highlighting to rainbow, loaded stuff, saw rainbow stick.
- Used "Show Entire File" in 1-up and 2-up views.
- Saw loading chrome.
- No loading chrome normally.
- Made inlines, verified `copyRows()` code runs.
- Poked around Diffusion -- it is missing some parameter handling, but works OK.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D11977
2015-03-05 23:03:00 +01:00
|
|
|
$parser->readParametersFromRequest($request);
|
2013-01-14 23:20:35 +01:00
|
|
|
|
2012-06-15 09:53:26 +02:00
|
|
|
if ($left && $right) {
|
|
|
|
$parser->setOriginals($left, $right);
|
|
|
|
}
|
|
|
|
|
2015-04-21 00:06:20 +02:00
|
|
|
$diff = $changeset->getDiff();
|
|
|
|
$revision_id = $diff->getRevisionID();
|
|
|
|
|
|
|
|
$can_mark = false;
|
|
|
|
$object_owner_phid = null;
|
|
|
|
if ($revision_id) {
|
|
|
|
$revision = id(new DifferentialRevisionQuery())
|
|
|
|
->setViewer($viewer)
|
|
|
|
->withIDs(array($revision_id))
|
|
|
|
->executeOne();
|
|
|
|
if ($revision) {
|
|
|
|
$can_mark = ($revision->getAuthorPHID() == $viewer->getPHID());
|
|
|
|
$object_owner_phid = $revision->getAuthorPHID();
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2011-05-09 06:22:25 +02:00
|
|
|
// Load both left-side and right-side inline comments.
|
2015-04-21 00:06:20 +02:00
|
|
|
if ($revision) {
|
|
|
|
$inlines = $this->loadInlineComments(
|
|
|
|
$revision,
|
2015-04-21 15:05:30 +02:00
|
|
|
array(nonempty($left, $right)),
|
2015-04-21 00:06:20 +02:00
|
|
|
$left_new,
|
2015-04-21 15:05:30 +02:00
|
|
|
array($right),
|
2015-04-21 00:06:20 +02:00
|
|
|
$right_new);
|
|
|
|
} else {
|
|
|
|
$inlines = array();
|
|
|
|
}
|
2011-05-09 06:22:25 +02:00
|
|
|
|
2012-01-04 18:10:37 +01:00
|
|
|
if ($left_new) {
|
|
|
|
$inlines = array_merge(
|
|
|
|
$inlines,
|
|
|
|
$this->buildLintInlineComments($left));
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($right_new) {
|
|
|
|
$inlines = array_merge(
|
|
|
|
$inlines,
|
|
|
|
$this->buildLintInlineComments($right));
|
|
|
|
}
|
|
|
|
|
2011-02-02 01:42:36 +01:00
|
|
|
$phids = array();
|
|
|
|
foreach ($inlines as $inline) {
|
|
|
|
$parser->parseInlineComment($inline);
|
2012-01-04 18:10:37 +01:00
|
|
|
if ($inline->getAuthorPHID()) {
|
|
|
|
$phids[$inline->getAuthorPHID()] = true;
|
|
|
|
}
|
2011-02-02 01:42:36 +01:00
|
|
|
}
|
|
|
|
$phids = array_keys($phids);
|
2011-02-02 22:48:52 +01:00
|
|
|
|
2012-09-05 04:02:56 +02:00
|
|
|
$handles = $this->loadViewerHandles($phids);
|
2011-02-02 01:42:36 +01:00
|
|
|
$parser->setHandles($handles);
|
2011-02-02 22:48:52 +01:00
|
|
|
|
2012-10-24 02:33:58 +02:00
|
|
|
$engine = new PhabricatorMarkupEngine();
|
Track a "Done" state on inline comments
Summary:
Ref T1460. This just barely works, but throwing it up in case any of it sounds mechanically crazy before we build integrations/UI/etc.
Specifically, these are the behaviors:
- You can mark your own draft comments as "done" before you submit them. The intent is to let reviewers mark their stuff advisory/minor/not-important before they submit it, to hint to authors that they don't expect the feedback to necessarily be addressed (maybe it's a joke, maybe it's just discussion, maybe it's "consider..").
- You can mark others' published comments as "done" if you're the revision/commit author. The intent is to keep this lightweight by not requiring an audit trail of who marked what done when. If anyone could mark anything done, we'd have to have some way to show who marked stuff.
- When you mark stuff done (or unmark it), it goes into a "draft" state, where you see the change but others don't see it yet. The intent is twofold:
- Be consistent with how inlines work.
- Allow us to publish a "epriestley updated this revision + epriestley marked 15 inlines as done" story later if we want. This seems more useful than publishing 15 "epriestley marked one thing as done" stories.
- The actual bit where done-ness publishes isn't implemented.
- UI is bare bones.
- No integration with the rest of the UI yet.
Test Plan: Clicked some checkboxes.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: paulshen, chasemp, epriestley
Maniphest Tasks: T1460
Differential Revision: https://secure.phabricator.com/D12033
2015-03-10 02:41:47 +01:00
|
|
|
$engine->setViewer($viewer);
|
2012-10-24 02:33:58 +02:00
|
|
|
|
|
|
|
foreach ($inlines as $inline) {
|
|
|
|
$engine->addObject(
|
|
|
|
$inline,
|
|
|
|
PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
|
|
|
|
}
|
|
|
|
|
|
|
|
$engine->process();
|
2011-01-25 00:52:35 +01:00
|
|
|
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
$parser
|
Track a "Done" state on inline comments
Summary:
Ref T1460. This just barely works, but throwing it up in case any of it sounds mechanically crazy before we build integrations/UI/etc.
Specifically, these are the behaviors:
- You can mark your own draft comments as "done" before you submit them. The intent is to let reviewers mark their stuff advisory/minor/not-important before they submit it, to hint to authors that they don't expect the feedback to necessarily be addressed (maybe it's a joke, maybe it's just discussion, maybe it's "consider..").
- You can mark others' published comments as "done" if you're the revision/commit author. The intent is to keep this lightweight by not requiring an audit trail of who marked what done when. If anyone could mark anything done, we'd have to have some way to show who marked stuff.
- When you mark stuff done (or unmark it), it goes into a "draft" state, where you see the change but others don't see it yet. The intent is twofold:
- Be consistent with how inlines work.
- Allow us to publish a "epriestley updated this revision + epriestley marked 15 inlines as done" story later if we want. This seems more useful than publishing 15 "epriestley marked one thing as done" stories.
- The actual bit where done-ness publishes isn't implemented.
- UI is bare bones.
- No integration with the rest of the UI yet.
Test Plan: Clicked some checkboxes.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: paulshen, chasemp, epriestley
Maniphest Tasks: T1460
Differential Revision: https://secure.phabricator.com/D12033
2015-03-10 02:41:47 +01:00
|
|
|
->setUser($viewer)
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
->setMarkupEngine($engine)
|
|
|
|
->setShowEditAndReplyLinks(true)
|
Track a "Done" state on inline comments
Summary:
Ref T1460. This just barely works, but throwing it up in case any of it sounds mechanically crazy before we build integrations/UI/etc.
Specifically, these are the behaviors:
- You can mark your own draft comments as "done" before you submit them. The intent is to let reviewers mark their stuff advisory/minor/not-important before they submit it, to hint to authors that they don't expect the feedback to necessarily be addressed (maybe it's a joke, maybe it's just discussion, maybe it's "consider..").
- You can mark others' published comments as "done" if you're the revision/commit author. The intent is to keep this lightweight by not requiring an audit trail of who marked what done when. If anyone could mark anything done, we'd have to have some way to show who marked stuff.
- When you mark stuff done (or unmark it), it goes into a "draft" state, where you see the change but others don't see it yet. The intent is twofold:
- Be consistent with how inlines work.
- Allow us to publish a "epriestley updated this revision + epriestley marked 15 inlines as done" story later if we want. This seems more useful than publishing 15 "epriestley marked one thing as done" stories.
- The actual bit where done-ness publishes isn't implemented.
- UI is bare bones.
- No integration with the rest of the UI yet.
Test Plan: Clicked some checkboxes.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: paulshen, chasemp, epriestley
Maniphest Tasks: T1460
Differential Revision: https://secure.phabricator.com/D12033
2015-03-10 02:41:47 +01:00
|
|
|
->setCanMarkDone($can_mark)
|
2015-03-27 19:23:10 +01:00
|
|
|
->setObjectOwnerPHID($object_owner_phid)
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
->setRange($range_s, $range_e)
|
|
|
|
->setMask($mask);
|
2012-03-13 01:06:55 +01:00
|
|
|
|
2011-01-25 20:57:47 +01:00
|
|
|
if ($request->isAjax()) {
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
$mcov = $parser->renderModifiedCoverage();
|
|
|
|
|
2012-03-13 05:39:05 +01:00
|
|
|
$coverage = array(
|
|
|
|
'differential-mcoverage-'.md5($changeset->getFilename()) => $mcov,
|
2012-03-13 01:06:55 +01:00
|
|
|
);
|
2012-03-13 05:39:05 +01:00
|
|
|
|
|
|
|
return id(new PhabricatorChangesetResponse())
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
->setRenderedChangeset($parser->renderChangeset())
|
2015-03-08 23:27:16 +01:00
|
|
|
->setCoverage($coverage)
|
|
|
|
->setUndoTemplates($parser->getRenderer()->renderUndoTemplates());
|
2011-01-25 20:57:47 +01:00
|
|
|
}
|
|
|
|
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
$detail = id(new DifferentialChangesetListView())
|
2015-03-05 23:01:15 +01:00
|
|
|
->setUser($this->getViewer())
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
->setChangesets(array($changeset))
|
|
|
|
->setVisibleChangesets(array($changeset))
|
|
|
|
->setRenderingReferences(array($rendering_reference))
|
Make "Show Context" persist rendering, whitespace, encoding, etc
Summary:
Ref T2009. Currently, we do not persist view parameters when making context rendering requests.
The big one is the renderer (1up vs 2up). This makes context on unified diffs come in with too many columns.
However, it impacts other parameters too. For example, at HEAD, if you change highlighting to "rainbow" and then load more context, the context uses the original highlighter instead of the rainbow highlighter.
This moves context loads into ChangesetViewManager, which maintains view parameters and can provide them correctly.
- This removes "ref"; it is no longer required, as the ChangesetViewManager tracks it.
- This removes URI management from `behavior-show-more`; it is no longer required, since the ChangesetViewManager knows how to render.
- This removes "whitespace" since this is handled properly by the view manager.
Test Plan:
- Used "Show Top" / "Show All" / "Show Bottom" in 1-up and 2-up views.
- Changed file highlighting to rainbow, loaded stuff, saw rainbow stick.
- Used "Show Entire File" in 1-up and 2-up views.
- Saw loading chrome.
- No loading chrome normally.
- Made inlines, verified `copyRows()` code runs.
- Poked around Diffusion -- it is missing some parameter handling, but works OK.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2009
Differential Revision: https://secure.phabricator.com/D11977
2015-03-05 23:03:00 +01:00
|
|
|
->setRenderURI('/differential/changeset/')
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
->setDiff($diff)
|
|
|
|
->setTitle(pht('Standalone View'))
|
|
|
|
->setParser($parser);
|
2011-05-06 21:58:53 +02:00
|
|
|
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
if ($revision_id) {
|
|
|
|
$detail->setInlineCommentControllerURI(
|
|
|
|
'/differential/comment/inline/edit/'.$revision_id.'/');
|
|
|
|
}
|
2011-01-25 00:52:35 +01:00
|
|
|
|
2014-05-25 17:59:31 +02:00
|
|
|
$crumbs = $this->buildApplicationCrumbs();
|
|
|
|
|
|
|
|
if ($revision_id) {
|
|
|
|
$crumbs->addTextCrumb('D'.$revision_id, '/D'.$revision_id);
|
|
|
|
}
|
|
|
|
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
$diff_id = $diff->getID();
|
2014-05-25 17:59:31 +02:00
|
|
|
if ($diff_id) {
|
|
|
|
$crumbs->addTextCrumb(
|
|
|
|
pht('Diff %d', $diff_id),
|
|
|
|
$this->getApplicationURI('diff/'.$diff_id));
|
|
|
|
}
|
|
|
|
|
|
|
|
$crumbs->addTextCrumb($changeset->getDisplayFilename());
|
|
|
|
|
Use ApplicationSearch in Differential
Summary:
Ref T603. Ref T2625. Fixes T3241. Depends on D5451. Depends on D6346.
@wez, this changes the Differential revision list UI substantially and may generate a lot of bikeshedding / who-moved-my-cheese churn. See T3417 for context, for example. The motivations for this change are:
- The list now works on devices, like phones and tablets. This is a requirement to make the rest of Differential work on devices.
- Although ApplicationSearch intentionally presents a simpler interface initially and some options which were one click away before aren't now, it is much more powerful than the search it replaces and allows users to build, save, share, fork, edit, and customize a much wider range of queries. Users who used the old filters frequently can use Advanced Search -> Save Custom Query to create new versions of them, and of any other query. "Edit Queries.." allows users to remove and reorder queries, including builtin queries. Basically, there are like three things which have gone from "1-click" to "a few clicks", and ten trillion things which have gone from "hard/impossible" to "relatively easy".
The local screenshots look a bit iffy, but I think a lot of this is the fakenesss of my test data. If they still feel iffy in production we can tweak them until they feel good, like we did for Maniphest.
Test Plan:
{F48477}
{F48478}
Reviewers: btrahan, chad, wez
Reviewed By: btrahan
CC: aran, s
Maniphest Tasks: T603, T2625, T3241
Differential Revision: https://secure.phabricator.com/D6347
2013-07-03 15:11:07 +02:00
|
|
|
return $this->buildApplicationPage(
|
2011-01-25 00:52:35 +01:00
|
|
|
array(
|
2014-05-25 17:59:31 +02:00
|
|
|
$crumbs,
|
Use ChangesetListView on Differential standalone view
Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).
Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:
- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView
I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.
Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4452, T2009
Differential Revision: https://secure.phabricator.com/D12012
2015-03-08 12:10:11 +01:00
|
|
|
$detail,
|
2011-01-25 00:52:35 +01:00
|
|
|
),
|
|
|
|
array(
|
2013-01-24 19:46:47 +01:00
|
|
|
'title' => pht('Changeset View'),
|
2014-06-24 00:15:11 +02:00
|
|
|
'device' => false,
|
2011-01-25 00:52:35 +01:00
|
|
|
));
|
|
|
|
}
|
|
|
|
|
2015-04-21 00:06:20 +02:00
|
|
|
private function loadInlineComments(
|
|
|
|
DifferentialRevision $revision,
|
2015-04-21 15:05:30 +02:00
|
|
|
array $left,
|
2015-04-21 00:06:20 +02:00
|
|
|
$left_new,
|
2015-04-21 15:05:30 +02:00
|
|
|
array $right,
|
2015-04-21 00:06:20 +02:00
|
|
|
$right_new) {
|
2011-05-09 06:22:25 +02:00
|
|
|
|
2015-04-21 15:05:30 +02:00
|
|
|
assert_instances_of($left, 'DifferentialChangeset');
|
|
|
|
assert_instances_of($right, 'DifferentialChangeset');
|
|
|
|
|
2015-04-21 00:06:20 +02:00
|
|
|
$viewer = $this->getViewer();
|
2015-04-21 15:05:30 +02:00
|
|
|
$all = array_merge($left, $right);
|
2015-04-21 00:06:20 +02:00
|
|
|
|
|
|
|
$inlines = id(new DifferentialInlineCommentQuery())
|
|
|
|
->setViewer($viewer)
|
|
|
|
->withRevisionPHIDs(array($revision->getPHID()))
|
2013-06-21 21:54:56 +02:00
|
|
|
->execute();
|
2015-04-21 00:06:20 +02:00
|
|
|
|
|
|
|
$changeset_ids = mpull($inlines, 'getChangesetID');
|
|
|
|
$changeset_ids = array_unique($changeset_ids);
|
|
|
|
if ($changeset_ids) {
|
|
|
|
$changesets = id(new DifferentialChangesetQuery())
|
|
|
|
->setViewer($viewer)
|
|
|
|
->withIDs($changeset_ids)
|
|
|
|
->execute();
|
|
|
|
$changesets = mpull($changesets, null, 'getID');
|
|
|
|
} else {
|
|
|
|
$changesets = array();
|
|
|
|
}
|
|
|
|
$changesets += mpull($all, null, 'getID');
|
|
|
|
|
2015-04-21 14:36:20 +02:00
|
|
|
$id_map = array();
|
|
|
|
foreach ($all as $changeset) {
|
|
|
|
$id_map[$changeset->getID()] = $changeset->getID();
|
|
|
|
}
|
2015-04-21 00:06:20 +02:00
|
|
|
|
2015-04-21 15:05:30 +02:00
|
|
|
// Generate filename maps for older and newer comments. If we're bringing
|
|
|
|
// an older comment forward in a diff-of-diffs, we want to put it on the
|
|
|
|
// left side of the screen, not the right side. Both sides are "new" files
|
|
|
|
// with the same name, so they're both appropriate targets, but the left
|
|
|
|
// is a better target conceptually for users because it's more consistent
|
|
|
|
// with the rest of the UI, which shows old information on the left and
|
|
|
|
// new information on the right.
|
2015-04-21 17:26:00 +02:00
|
|
|
$move_here = DifferentialChangeType::TYPE_MOVE_HERE;
|
|
|
|
|
2015-04-21 15:05:30 +02:00
|
|
|
$name_map_old = array();
|
|
|
|
$name_map_new = array();
|
2015-04-21 17:26:00 +02:00
|
|
|
$move_map = array();
|
2015-04-21 14:36:20 +02:00
|
|
|
foreach ($all as $changeset) {
|
2015-04-21 15:05:30 +02:00
|
|
|
$changeset_id = $changeset->getID();
|
|
|
|
|
2015-04-21 17:26:00 +02:00
|
|
|
$filenames = array();
|
|
|
|
$filenames[] = $changeset->getFilename();
|
|
|
|
|
|
|
|
// If this is the target of a move, also map comments on the old filename
|
|
|
|
// to this changeset.
|
|
|
|
if ($changeset->getChangeType() == $move_here) {
|
|
|
|
$old_file = $changeset->getOldFile();
|
|
|
|
$filenames[] = $old_file;
|
|
|
|
$move_map[$changeset_id][$old_file] = true;
|
2015-04-21 15:05:30 +02:00
|
|
|
}
|
|
|
|
|
2015-04-21 17:26:00 +02:00
|
|
|
foreach ($filenames as $filename) {
|
|
|
|
// We update the old map only if we don't already have an entry (oldest
|
|
|
|
// changeset persists).
|
|
|
|
if (empty($name_map_old[$filename])) {
|
|
|
|
$name_map_old[$filename] = $changeset_id;
|
|
|
|
}
|
|
|
|
|
|
|
|
// We always update the new map (newest changeset overwrites).
|
|
|
|
$name_map_new[$changeset->getFilename()] = $changeset_id;
|
|
|
|
}
|
2015-04-21 14:36:20 +02:00
|
|
|
}
|
2015-04-21 00:06:20 +02:00
|
|
|
|
2015-04-21 15:05:30 +02:00
|
|
|
// Find the smallest "new" changeset ID. We'll consider everything
|
|
|
|
// larger than this to be "newer", and everything smaller to be "older".
|
|
|
|
$first_new_id = min(mpull($right, 'getID'));
|
|
|
|
|
2015-04-21 00:06:20 +02:00
|
|
|
$results = array();
|
|
|
|
foreach ($inlines as $inline) {
|
|
|
|
$changeset_id = $inline->getChangesetID();
|
|
|
|
if (isset($id_map[$changeset_id])) {
|
|
|
|
// This inline is legitimately on one of the current changesets, so
|
|
|
|
// we can include it in the result set unmodified.
|
|
|
|
$results[] = $inline;
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$changeset = idx($changesets, $changeset_id);
|
|
|
|
if (!$changeset) {
|
2015-04-21 14:36:20 +02:00
|
|
|
// Just discard this inline, as it has bogus data.
|
2015-04-21 00:06:20 +02:00
|
|
|
continue;
|
|
|
|
}
|
|
|
|
|
|
|
|
$target_id = null;
|
|
|
|
|
2015-04-21 15:05:30 +02:00
|
|
|
if ($changeset_id >= $first_new_id) {
|
|
|
|
$name_map = $name_map_new;
|
2015-04-21 15:16:03 +02:00
|
|
|
$is_new = true;
|
2015-04-21 15:05:30 +02:00
|
|
|
} else {
|
|
|
|
$name_map = $name_map_old;
|
2015-04-21 15:16:03 +02:00
|
|
|
$is_new = false;
|
2015-04-21 15:05:30 +02:00
|
|
|
}
|
|
|
|
|
2015-04-21 00:06:20 +02:00
|
|
|
$filename = $changeset->getFilename();
|
|
|
|
if (isset($name_map[$filename])) {
|
|
|
|
// This changeset is on a file with the same name as the current
|
|
|
|
// changeset, so we're going to port it forward or backward.
|
|
|
|
$target_id = $name_map[$filename];
|
2015-04-21 17:26:00 +02:00
|
|
|
|
|
|
|
$is_move = isset($move_map[$target_id][$filename]);
|
2015-04-21 15:16:03 +02:00
|
|
|
if ($is_new) {
|
2015-04-21 17:26:00 +02:00
|
|
|
if ($is_move) {
|
|
|
|
$reason = pht(
|
|
|
|
'This comment was made on a file with the same name as the '.
|
|
|
|
'file this file was moved from, but in a newer diff.');
|
|
|
|
} else {
|
|
|
|
$reason = pht(
|
|
|
|
'This comment was made on a file with the same name, but '.
|
|
|
|
'in a newer diff.');
|
|
|
|
}
|
2015-04-21 15:16:03 +02:00
|
|
|
} else {
|
2015-04-21 17:26:00 +02:00
|
|
|
if ($is_move) {
|
|
|
|
$reason = pht(
|
|
|
|
'This comment was made on a file with the same name as the '.
|
|
|
|
'file this file was moved from, but in an older diff.');
|
|
|
|
} else {
|
|
|
|
$reason = pht(
|
|
|
|
'This comment was made on a file with the same name, but '.
|
|
|
|
'in an older diff.');
|
|
|
|
}
|
2015-04-21 15:16:03 +02:00
|
|
|
}
|
2015-04-21 00:06:20 +02:00
|
|
|
}
|
|
|
|
|
2015-04-21 17:26:00 +02:00
|
|
|
|
|
|
|
// If we didn't find a target and this change is the target of a move,
|
|
|
|
// look for a match against the old filename.
|
|
|
|
if (!$target_id) {
|
|
|
|
if ($changeset->getChangeType() == $move_here) {
|
|
|
|
$filename = $changeset->getOldFile();
|
|
|
|
if (isset($name_map[$filename])) {
|
|
|
|
$target_id = $name_map[$filename];
|
|
|
|
if ($is_new) {
|
|
|
|
$reason = pht(
|
|
|
|
'This comment was made on a file which this file was moved '.
|
|
|
|
'to, but in a newer diff.');
|
|
|
|
} else {
|
|
|
|
$reason = pht(
|
|
|
|
'This comment was made on a file which this file was moved '.
|
|
|
|
'to, but in an older diff.');
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
|
2015-04-21 00:06:20 +02:00
|
|
|
// If we found a changeset to port this comment to, bring it forward
|
|
|
|
// or backward and mark it.
|
|
|
|
if ($target_id) {
|
|
|
|
$inline
|
|
|
|
->makeEphemeral(true)
|
|
|
|
->setChangesetID($target_id)
|
2015-04-21 15:16:03 +02:00
|
|
|
->setIsGhost(
|
|
|
|
array(
|
|
|
|
'new' => $is_new,
|
|
|
|
'reason' => $reason,
|
|
|
|
));
|
2015-04-21 00:06:20 +02:00
|
|
|
|
|
|
|
$results[] = $inline;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2015-04-21 14:36:20 +02:00
|
|
|
// Filter out the inlines we ported forward which won't be visible because
|
|
|
|
// they appear on the wrong side of a file.
|
|
|
|
$keep_map = array();
|
2015-04-21 15:05:30 +02:00
|
|
|
foreach ($left as $changeset) {
|
|
|
|
$keep_map[$changeset->getID()][(int)$left_new] = true;
|
|
|
|
}
|
|
|
|
foreach ($right as $changeset) {
|
|
|
|
$keep_map[$changeset->getID()][(int)$right_new] = true;
|
|
|
|
}
|
2015-04-21 14:36:20 +02:00
|
|
|
foreach ($results as $key => $inline) {
|
|
|
|
$is_new = (int)$inline->getIsNewFile();
|
|
|
|
$changeset_id = $inline->getChangesetID();
|
|
|
|
if (!isset($keep_map[$changeset_id][$is_new])) {
|
|
|
|
unset($results[$key]);
|
|
|
|
continue;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2015-04-21 00:06:20 +02:00
|
|
|
return $results;
|
2011-02-02 01:42:36 +01:00
|
|
|
}
|
|
|
|
|
2012-02-15 02:00:20 +01:00
|
|
|
private function buildRawFileResponse(
|
|
|
|
DifferentialChangeset $changeset,
|
|
|
|
$is_new) {
|
|
|
|
|
Track a "Done" state on inline comments
Summary:
Ref T1460. This just barely works, but throwing it up in case any of it sounds mechanically crazy before we build integrations/UI/etc.
Specifically, these are the behaviors:
- You can mark your own draft comments as "done" before you submit them. The intent is to let reviewers mark their stuff advisory/minor/not-important before they submit it, to hint to authors that they don't expect the feedback to necessarily be addressed (maybe it's a joke, maybe it's just discussion, maybe it's "consider..").
- You can mark others' published comments as "done" if you're the revision/commit author. The intent is to keep this lightweight by not requiring an audit trail of who marked what done when. If anyone could mark anything done, we'd have to have some way to show who marked stuff.
- When you mark stuff done (or unmark it), it goes into a "draft" state, where you see the change but others don't see it yet. The intent is twofold:
- Be consistent with how inlines work.
- Allow us to publish a "epriestley updated this revision + epriestley marked 15 inlines as done" story later if we want. This seems more useful than publishing 15 "epriestley marked one thing as done" stories.
- The actual bit where done-ness publishes isn't implemented.
- UI is bare bones.
- No integration with the rest of the UI yet.
Test Plan: Clicked some checkboxes.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: paulshen, chasemp, epriestley
Maniphest Tasks: T1460
Differential Revision: https://secure.phabricator.com/D12033
2015-03-10 02:41:47 +01:00
|
|
|
$viewer = $this->getViewer();
|
2013-09-30 18:38:13 +02:00
|
|
|
|
2012-02-15 02:00:20 +01:00
|
|
|
if ($is_new) {
|
|
|
|
$key = 'raw:new:phid';
|
|
|
|
} else {
|
|
|
|
$key = 'raw:old:phid';
|
|
|
|
}
|
|
|
|
|
|
|
|
$metadata = $changeset->getMetadata();
|
|
|
|
|
|
|
|
$file = null;
|
|
|
|
$phid = idx($metadata, $key);
|
|
|
|
if ($phid) {
|
2013-09-30 18:38:13 +02:00
|
|
|
$file = id(new PhabricatorFileQuery())
|
|
|
|
->setViewer($viewer)
|
|
|
|
->withPHIDs(array($phid))
|
|
|
|
->execute();
|
|
|
|
if ($file) {
|
|
|
|
$file = head($file);
|
|
|
|
}
|
2012-02-15 02:00:20 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
if (!$file) {
|
|
|
|
// This is just building a cache of the changeset content in the file
|
|
|
|
// tool, and is safe to run on a read pathway.
|
|
|
|
$unguard = AphrontWriteGuard::beginScopedUnguardedWrites();
|
|
|
|
|
|
|
|
if ($is_new) {
|
|
|
|
$data = $changeset->makeNewFile();
|
|
|
|
} else {
|
|
|
|
$data = $changeset->makeOldFile();
|
|
|
|
}
|
|
|
|
|
|
|
|
$file = PhabricatorFile::newFromFileData(
|
|
|
|
$data,
|
|
|
|
array(
|
|
|
|
'name' => $changeset->getFilename(),
|
|
|
|
'mime-type' => 'text/plain',
|
|
|
|
));
|
|
|
|
|
|
|
|
$metadata[$key] = $file->getPHID();
|
|
|
|
$changeset->setMetadata($metadata);
|
|
|
|
$changeset->save();
|
|
|
|
|
|
|
|
unset($unguard);
|
|
|
|
}
|
|
|
|
|
2014-08-20 00:53:15 +02:00
|
|
|
return $file->getRedirectResponse();
|
2011-05-06 21:58:53 +02:00
|
|
|
}
|
2011-02-02 01:42:36 +01:00
|
|
|
|
2012-01-04 18:10:37 +01:00
|
|
|
private function buildLintInlineComments($changeset) {
|
|
|
|
$lint = id(new DifferentialDiffProperty())->loadOneWhere(
|
|
|
|
'diffID = %d AND name = %s',
|
|
|
|
$changeset->getDiffID(),
|
|
|
|
'arc:lint');
|
|
|
|
if (!$lint) {
|
2012-01-05 18:11:49 +01:00
|
|
|
return array();
|
2012-01-04 18:10:37 +01:00
|
|
|
}
|
|
|
|
$lint = $lint->getData();
|
|
|
|
|
|
|
|
$inlines = array();
|
|
|
|
foreach ($lint as $msg) {
|
2012-01-17 08:05:44 +01:00
|
|
|
if ($msg['path'] != $changeset->getFilename()) {
|
2012-01-04 18:10:37 +01:00
|
|
|
continue;
|
|
|
|
}
|
|
|
|
$inline = new DifferentialInlineComment();
|
|
|
|
$inline->setChangesetID($changeset->getID());
|
2014-02-21 20:52:03 +01:00
|
|
|
$inline->setIsNewFile(1);
|
2012-01-04 18:10:37 +01:00
|
|
|
$inline->setSyntheticAuthor('Lint: '.$msg['name']);
|
|
|
|
$inline->setLineNumber($msg['line']);
|
|
|
|
$inline->setLineLength(0);
|
|
|
|
|
|
|
|
$inline->setContent('%%%'.$msg['description'].'%%%');
|
|
|
|
|
|
|
|
$inlines[] = $inline;
|
|
|
|
}
|
|
|
|
|
|
|
|
return $inlines;
|
|
|
|
}
|
|
|
|
|
2011-01-25 00:52:35 +01:00
|
|
|
}
|