diff --git a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php b/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php index 558f747d7c..f9cd71b188 100644 --- a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php @@ -22,15 +22,91 @@ class DifferentialChangesetViewController extends DifferentialController { public function processRequest() { $request = $this->getRequest(); - $id = $request->getStr('id'); $author_phid = $request->getUser()->getPHID(); + $id = $request->getStr('id'); + $vs = $request->getInt('vs'); + + $changeset = id(new DifferentialChangeset())->load($id); if (!$changeset) { return new Aphront404Response(); } - $changeset->attachHunks($changeset->loadHunks()); + if ($vs && ($vs != -1)) { + $vs_changeset = id(new DifferentialChangeset())->load($vs); + if (!$vs_changeset) { + return new Aphront404Response(); + } + } + + if (!$vs) { + $right = $changeset; + $left = null; + + $right_source = $right->getID(); + $right_new = true; + $left_source = $right->getID(); + $left_new = false; + } else if ($vs == -1) { + $right = null; + $left = $changeset; + + $right_source = $left->getID(); + $right_new = false; + $left_source = $left->getID(); + $left_new = true; + } else { + $right = $changeset; + $left = $vs_changeset; + + $right_source = $right->getID(); + $right_new = true; + $left_source = $left->getID(); + $left_new = true; + } + + if ($left) { + $left->attachHunks($left->loadHunks()); + } + + if ($right) { + $right->attachHunks($right->loadHunks()); + } + + if ($left) { + + $left_data = $left->makeNewFile(); + if ($right) { + $right_data = $right->makeNewFile(); + } else { + $right_data = $left->makeOldFile(); + } + + $left_tmp = new TempFile(); + $right_tmp = new TempFile(); + Filesystem::writeFile($left_tmp, $left_data); + Filesystem::writeFile($right_tmp, $right_data); + list($err, $stdout) = exec_manual( + '/usr/bin/diff -U65535 %s %s', + $left_tmp, + $right_tmp); + + $choice = nonempty($left, $right); + if ($stdout) { + $parser = new ArcanistDiffParser(); + $changes = $parser->parseDiff($stdout); + $diff = DifferentialDiff::newFromRawChanges($changes); + $changesets = $diff->getChangesets(); + $first = reset($changesets); + $choice->attachHunks($first->getHunks()); + } else { + $choice->attachHunks(array()); + } + + $changeset = $choice; + $changeset->setID(null); + } $range_s = null; $range_e = null; @@ -54,6 +130,8 @@ class DifferentialChangesetViewController extends DifferentialController { $parser = new DifferentialChangesetParser(); $parser->setChangeset($changeset); + $parser->setRightSideCommentMapping($right_source, $right_new); + $parser->setLeftSideCommentMapping($left_source, $left_new); $phids = array(); $inlines = $this->loadInlineComments($id, $author_phid); diff --git a/src/applications/differential/controller/changesetview/__init__.php b/src/applications/differential/controller/changesetview/__init__.php index 124a9dc4e0..3c23c13098 100644 --- a/src/applications/differential/controller/changesetview/__init__.php +++ b/src/applications/differential/controller/changesetview/__init__.php @@ -6,17 +6,23 @@ +phutil_require_module('arcanist', 'parser/diff'); + phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'aphront/response/ajax'); phutil_require_module('phabricator', 'applications/differential/controller/base'); phutil_require_module('phabricator', 'applications/differential/parser/changeset'); phutil_require_module('phabricator', 'applications/differential/parser/markup'); phutil_require_module('phabricator', 'applications/differential/storage/changeset'); +phutil_require_module('phabricator', 'applications/differential/storage/diff'); phutil_require_module('phabricator', 'applications/differential/storage/inlinecomment'); phutil_require_module('phabricator', 'applications/differential/view/changesetdetailview'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); +phutil_require_module('phutil', 'filesystem'); +phutil_require_module('phutil', 'filesystem/tempfile'); +phutil_require_module('phutil', 'future/exec'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/differential/controller/diffview/DifferentialDiffViewController.php b/src/applications/differential/controller/diffview/DifferentialDiffViewController.php index db37db9308..9f13d22d3b 100644 --- a/src/applications/differential/controller/diffview/DifferentialDiffViewController.php +++ b/src/applications/differential/controller/diffview/DifferentialDiffViewController.php @@ -40,6 +40,35 @@ class DifferentialDiffViewController extends DifferentialController { 'When you are satisfied, either create a new revision '. 'or update an existing revision.'); + // TODO: implmenent optgroup support in AphrontFormSelectControl? + $select = array(); + $select[] = ''; + $select[] = ''; + $select[] = ''; + + $revision_data = new DifferentialRevisionListData( + DifferentialRevisionListData::QUERY_OPEN_OWNED, + array($request->getUser()->getPHID())); + $revisions = $revision_data->loadRevisions(); + + if ($revisions) { + $select[] = ''; + foreach ($revisions as $revision) { + $select[] = phutil_render_tag( + 'option', + array( + 'value' => $revision->getID(), + ), + phutil_escape_html($revision->getTitle())); + } + $select[] = ''; + } + + $select = + ''; + $action_form = new AphrontFormView(); $action_form ->setUser($request->getUser()) @@ -47,13 +76,9 @@ class DifferentialDiffViewController extends DifferentialController { ->addHiddenInput('diffID', $diff->getID()) ->addHiddenInput('viaDiffView', 1) ->appendChild( - id(new AphrontFormSelectControl()) + id(new AphrontFormMarkupControl()) ->setLabel('Attach To') - ->setName('revisionID') - ->setValue('') - ->setOptions(array( - '' => "Create a new Revision...", - ))) + ->setValue($select)) ->appendChild( id(new AphrontFormSubmitControl()) ->setValue('Continue')); diff --git a/src/applications/differential/controller/diffview/__init__.php b/src/applications/differential/controller/diffview/__init__.php index efcd27da68..b685510625 100644 --- a/src/applications/differential/controller/diffview/__init__.php +++ b/src/applications/differential/controller/diffview/__init__.php @@ -8,6 +8,7 @@ phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'applications/differential/controller/base'); +phutil_require_module('phabricator', 'applications/differential/data/revisionlist'); phutil_require_module('phabricator', 'applications/differential/storage/diff'); phutil_require_module('phabricator', 'applications/differential/view/changesetlistview'); phutil_require_module('phabricator', 'applications/differential/view/difftableofcontents'); @@ -15,6 +16,7 @@ phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/layout/panel'); +phutil_require_module('phutil', 'markup'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php b/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php index 8d85e520a9..5f4822db12 100644 --- a/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php +++ b/src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php @@ -26,6 +26,12 @@ class DifferentialRevisionEditController extends DifferentialController { public function processRequest() { + $request = $this->getRequest(); + + if (!$this->id) { + $this->id = $request->getInt('revisionID'); + } + if ($this->id) { $revision = id(new DifferentialRevision())->load($this->id); if (!$revision) { @@ -35,7 +41,6 @@ class DifferentialRevisionEditController extends DifferentialController { $revision = new DifferentialRevision(); } - $request = $this->getRequest(); $diff_id = $request->getInt('diffID'); if ($diff_id) { $diff = id(new DifferentialDiff())->load($diff_id); diff --git a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php index 56ab73edb1..b92c29e98f 100644 --- a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php @@ -37,10 +37,16 @@ class DifferentialRevisionViewController extends DifferentialController { $diffs = $revision->loadDiffs(); - $diff_vs = null; + $diff_vs = $request->getInt('vs'); $target = end($diffs); - $changesets = $target->loadChangesets(); + $diffs = mpull($diffs, null, 'getID'); + if (empty($diffs[$diff_vs])) { + $diff_vs = null; + } + + list($changesets, $vs_map) = + $this->loadChangesetsAndVsMap($diffs, $diff_vs, $target); $comments = $revision->loadComments(); $comments = array_merge( @@ -89,6 +95,7 @@ class DifferentialRevisionViewController extends DifferentialController { $changeset_view->setChangesets($changesets); $changeset_view->setEditable(true); $changeset_view->setRevision($revision); + $changeset_view->setVsMap($vs_map); $comment_form = new DifferentialAddCommentView(); $comment_form->setRevision($revision); @@ -315,6 +322,44 @@ class DifferentialRevisionViewController extends DifferentialController { return $inline_comments; } + private function loadChangesetsAndVsMap(array $diffs, $diff_vs, $target) { + $load_ids = array(); + if ($diff_vs) { + $load_ids[] = $diff_vs; + } + $load_ids[] = $target->getID(); + + $raw_changesets = id(new DifferentialChangeset()) + ->loadAllWhere( + 'diffID IN (%Ld)', + $load_ids); + $changeset_groups = mgroup($raw_changesets, 'getDiffID'); + + $changesets = idx($changeset_groups, $target->getID(), array()); + $changesets = mpull($changesets, null, 'getID'); + + $vs_map = array(); + if ($diff_vs) { + $vs_changesets = idx($changeset_groups, $diff_vs, array()); + $vs_changesets = mpull($vs_changesets, null, 'getFilename'); + foreach ($changesets as $key => $changeset) { + $file = $changeset->getFilename(); + if (isset($vs_changesets[$file])) { + $vs_map[$changeset->getID()] = $vs_changesets[$file]->getID(); + unset($vs_changesets[$file]); + } + } + foreach ($vs_changesets as $changeset) { + $changesets[$changeset->getID()] = $changeset; + $vs_map[$changeset->getID()] = -1; + } + } + + $changesets = msort($changesets, 'getSortKey'); + + return array($changesets, $vs_map); + } + } /* @@ -417,16 +462,6 @@ class DifferentialRevisionViewController extends DifferentialController { } } - $reviewer_links = array(); - foreach ($revision->getReviewers() as $reviewer) { - $reviewer_links[] = ; - } - if ($reviewer_links) { - $fields['Reviewers'] = array_implode(', ', $reviewer_links); - } else { - $fields['Reviewers'] = None; - } $ccs = $revision->getCCFBIDs(); if ($ccs) { @@ -777,68 +812,8 @@ class DifferentialRevisionViewController extends DifferentialController { $changesets = id(new DifferentialChangeset())->loadAllFromArray($objects); } - $against_warn = null; - $against_map = array(); - $visible_changesets = array(); - if ($old) { - $old_diff = $diffs[$old]; - $new_diff = $diff; - $old_path = $old_diff->getSourcePath(); - $new_path = $new_diff->getSourcePath(); - - $old_prefix = null; - $new_prefix = null; - if ((strlen($old_path) < strlen($new_path)) && - (!strncmp($old_path, $new_path, strlen($old_path)))) { - $old_prefix = substr($new_path, strlen($old_path)); - } - if ((strlen($new_path) < strlen($old_path)) && - (!strncmp($old_path, $new_path, strlen($new_path)))) { - $new_prefix = substr($old_path, strlen($new_path)); - } - - $old_changesets = id(new DifferentialChangeset()) - ->loadAllFromArray($raw_objects[$old]); - $old_changesets = array_pull($old_changesets, null, 'getFilename'); - if ($new_prefix) { - $rekeyed_map = array(); - foreach ($old_changesets as $key => $value) { - $rekeyed_map[$new_prefix.$key] = $value; - } - $old_changesets = $rekeyed_map; - } - - foreach ($changesets as $key => $changeset) { - $file = $old_prefix.$changeset->getFilename(); - if (isset($old_changesets[$file])) { - $checksum = $changeset->getChecksum(); - if ($checksum !== null && - $checksum == $old_changesets[$file]->getChecksum()) { - unset($changesets[$key]); - unset($old_changesets[$file]); - } else { - $against_map[$changeset->getID()] = $old_changesets[$file]->getID(); - unset($old_changesets[$file]); - } - } - } - - foreach ($old_changesets as $changeset) { - $changesets[$changeset->getID()] = $changeset; - $against_map[$changeset->getID()] = -1; - } - - $against_warn = - - You are viewing a synthetic diff between two previous diffs in this - revision. You can not add new inline comments (for now). - ; - } else { - $visible_changesets = array_pull($changesets, 'getID'); - } $changesets = array_psort($changesets, 'getSortKey'); - $all_changesets = $changesets; $warning = null; $limit = 100; diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php index 5fa6cb54bf..6e4de2f90d 100644 --- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php @@ -58,6 +58,13 @@ class DifferentialChangesetParser { const WHITESPACE_IGNORE_TRAILING = 'ignore-trailing'; const WHITESPACE_IGNORE_ALL = 'ignore-all'; + public function setRightSideCommentMapping($id, $is_new) { + + } + + public function setLeftSideCommentMapping($id, $is_new) { + + } public function setChangeset($changeset) { $this->changeset = $changeset; diff --git a/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php b/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php index a4b2261464..e05ae7a836 100644 --- a/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php +++ b/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php @@ -21,6 +21,7 @@ class DifferentialChangesetListView extends AphrontView { private $changesets = array(); private $editable; private $revision; + private $vsMap; public function setChangesets($changesets) { $this->changesets = $changesets; @@ -37,48 +38,39 @@ class DifferentialChangesetListView extends AphrontView { return $this; } + public function setVsMap(array $vs_map) { + $this->vsMap = $vs_map; + return $this; + } + public function render() { require_celerity_resource('differential-changeset-view-css'); - $against = array(); // TODO - $edit = false; - + $vs_map = $this->vsMap; $changesets = $this->changesets; - foreach ($changesets as $key => $changeset) { - if (empty($against[$changeset->getID()])) { - $type = $changeset->getChangeType(); - if ($type == DifferentialChangeType::TYPE_MOVE_AWAY || - $type == DifferentialChangeType::TYPE_MULTICOPY) { - unset($changesets[$key]); - } - } - } $output = array(); $mapping = array(); foreach ($changesets as $key => $changeset) { $file = $changeset->getFilename(); $class = 'differential-changeset'; - if (!$edit) { + if (!$this->editable) { $class .= ' differential-changeset-noneditable'; } $id = $changeset->getID(); if ($id) { - $against_id = idx($against, $id); + $vs_id = idx($vs_map, $id); } else { - $against_id = null; + $vs_id = null; } -/* - TODO - $detail_uri = URI($render_uri) - ->addQueryData(array( - 'changeset' => $id, - 'against' => $against_id, - 'whitespace' => $whitespace, + $detail_uri = new PhutilURI('/differential/changeset/'); + $detail_uri->setQueryParams( + array( + 'id' => $id, + 'vs' => $vs_id, + 'whitespace' => 'TODO', )); -*/ - $detail_uri = '/differential/changeset/?id='.$changeset->getID(); $detail_button = phutil_render_tag( 'a', @@ -104,7 +96,9 @@ class DifferentialChangesetListView extends AphrontView { '
Loading...
')); $output[] = $detail->render(); - $mapping[$uniq_id] = array($changeset->getID()); + $mapping[$uniq_id] = array( + $changeset->getID(), + $vs_id); } $whitespace = null; diff --git a/src/applications/differential/view/changesetlistview/__init__.php b/src/applications/differential/view/changesetlistview/__init__.php index 9f040ff17f..09a25e7875 100644 --- a/src/applications/differential/view/changesetlistview/__init__.php +++ b/src/applications/differential/view/changesetlistview/__init__.php @@ -6,13 +6,13 @@ -phutil_require_module('phabricator', 'applications/differential/constants/changetype'); phutil_require_module('phabricator', 'applications/differential/view/changesetdetailview'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'view/base'); phutil_require_module('phutil', 'markup'); +phutil_require_module('phutil', 'parser/uri'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/differential/view/revisionupdatehistory/DifferentialRevisionUpdateHistoryView.php b/src/applications/differential/view/revisionupdatehistory/DifferentialRevisionUpdateHistoryView.php index b54e1a4e04..17fc0d3e28 100644 --- a/src/applications/differential/view/revisionupdatehistory/DifferentialRevisionUpdateHistoryView.php +++ b/src/applications/differential/view/revisionupdatehistory/DifferentialRevisionUpdateHistoryView.php @@ -69,18 +69,38 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView { $idx = 0; $rows = array(); + $disable = false; + $radios = array(); foreach ($data as $row) { $name = phutil_escape_html($row['name']); $id = phutil_escape_html($row['id']); - $radios = array(); $lint = '*'; $unit = '*'; $old_class = null; $new_class = null; + if ($id) { + $new_checked = ($this->selectedDiffID == $id); + $new = javelin_render_tag( + 'input', + array( + 'type' => 'radio', + 'name' => 'id', + 'value' => $id, + 'checked' => $new_checked ? 'checked' : null, + 'sigil' => 'differential-new-radio', + )); + if ($new_checked) { + $new_class = " revhistory-new-now"; + $disable = true; + } + } else { + $new = null; + } + if ($max_id != $id) { $uniq = celerity_generate_unique_node_id(); $old_checked = ($this->selectedVersusDiffID == $id); @@ -89,9 +109,10 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView { array( 'type' => 'radio', 'name' => 'vs', - 'value' => '0', + 'value' => $id, 'id' => $uniq, 'checked' => $old_checked ? 'checked' : null, + 'disabled' => $disable ? 'disabled' : null, )); $radios[] = $uniq; if ($old_checked) { @@ -101,30 +122,6 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView { $old = null; } - if ($id) { - $new_checked = ($this->selectedDiffID == $id); - $new = phutil_render_tag( - 'input', - array( - 'type' => 'radio', - 'name' => 'id', - 'value' => $id, - 'checked' => $new_checked ? 'checked' : null, - )); - if ($new_checked) { - $new_class = " revhistory-new-now"; - } - } else { - $new = null; - } - - Javelin::initBehavior( - 'differential-diff-radios', - array( - 'radios' => $radios, - )); - - $desc = 'TODO'; $age = '-'; @@ -147,6 +144,12 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView { ''; } + Javelin::initBehavior( + 'differential-diff-radios', + array( + 'radios' => $radios, + )); + $select = ''; return diff --git a/src/applications/differential/view/revisionupdatehistory/__init__.php b/src/applications/differential/view/revisionupdatehistory/__init__.php index d615e831fa..ab2bc66cee 100644 --- a/src/applications/differential/view/revisionupdatehistory/__init__.php +++ b/src/applications/differential/view/revisionupdatehistory/__init__.php @@ -8,6 +8,7 @@ phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); +phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/base'); phutil_require_module('phutil', 'markup'); diff --git a/webroot/rsrc/css/core/core.css b/webroot/rsrc/css/core/core.css index 8fc344c0e1..bc6836e779 100644 --- a/webroot/rsrc/css/core/core.css +++ b/webroot/rsrc/css/core/core.css @@ -65,6 +65,12 @@ select, input, button, textarea, button { font: 99% 'lucida grande', tahoma, verdana, arial, clean, sans-serif; } +select { + /* NOTE: In Safari, specifying a border color makes the browser respect + "font-size" settings. */ + border: 1px solid #999999; +} + table { font-size: inherit; font: 100%; diff --git a/webroot/rsrc/js/application/differential/behavior-diff-radios.js b/webroot/rsrc/js/application/differential/behavior-diff-radios.js index b000736591..29ad8bbd09 100644 --- a/webroot/rsrc/js/application/differential/behavior-diff-radios.js +++ b/webroot/rsrc/js/application/differential/behavior-diff-radios.js @@ -6,7 +6,7 @@ JX.behavior('differential-diff-radios', function(config) { JX.Stratcom.listen( 'click', - 'new-radio', + 'differential-new-radio', function(e) { var target = e.getTarget(); var adjust; @@ -14,7 +14,7 @@ JX.behavior('differential-diff-radios', function(config) { var reset = false; for (var ii = 0; ii < config.radios.length; ii++) { node = JX.$(config.radios[ii]); - if (node.value >= target.value) { + if (parseInt(node.value, 10) >= parseInt(target.value, 10)) { if (node.checked) { node.checked = false; reset = true; diff --git a/webroot/rsrc/js/application/differential/behavior-populate.js b/webroot/rsrc/js/application/differential/behavior-populate.js index 6409521253..0712252c56 100644 --- a/webroot/rsrc/js/application/differential/behavior-populate.js +++ b/webroot/rsrc/js/application/differential/behavior-populate.js @@ -13,7 +13,7 @@ JX.behavior('differential-populate', function(config) { new JX.Request(config.uri, JX.bind(null, onresponse, k)) .setData({ id: config.registry[k][0], - against: config.registry[k][1], + vs: config.registry[k][1], whitespace: config.whitespace }) .send();