diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -11,8 +11,8 @@ 'conpherence.pkg.js' => '15191c65', 'core.pkg.css' => '641086ed', 'core.pkg.js' => '1ea38af8', - 'differential.pkg.css' => '113e692c', - 'differential.pkg.js' => '3da2650a', + 'differential.pkg.css' => '06dc617c', + 'differential.pkg.js' => 'c2ca903a', 'diffusion.pkg.css' => 'a2d17c7d', 'diffusion.pkg.js' => '6134c5a1', 'maniphest.pkg.css' => '4845691a', @@ -61,7 +61,7 @@ 'rsrc/css/application/dashboard/dashboard.css' => 'fe5b1869', 'rsrc/css/application/diff/inline-comment-summary.css' => 'f23d4e8f', 'rsrc/css/application/differential/add-comment.css' => 'c47f8c40', - 'rsrc/css/application/differential/changeset-view.css' => 'bf84345b', + 'rsrc/css/application/differential/changeset-view.css' => 'db34a142', 'rsrc/css/application/differential/core.css' => '5b7b8ff4', 'rsrc/css/application/differential/phui-inline-comment.css' => '65ae3bc2', 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', @@ -71,7 +71,6 @@ 'rsrc/css/application/diffusion/diffusion-icons.css' => '0c15255e', 'rsrc/css/application/diffusion/diffusion-readme.css' => '419dd5b6', 'rsrc/css/application/diffusion/diffusion-repository.css' => 'ee6f20ec', - 'rsrc/css/application/diffusion/diffusion-source.css' => '5f35a3bd', 'rsrc/css/application/diffusion/diffusion.css' => '45727264', 'rsrc/css/application/feed/feed.css' => 'ecd4ec57', 'rsrc/css/application/files/global-drag-and-drop.css' => 'b556a948', @@ -120,7 +119,7 @@ 'rsrc/css/font/font-lato.css' => 'c7ccd872', 'rsrc/css/font/phui-font-icon-base.css' => '870a7360', 'rsrc/css/layout/phabricator-filetree-view.css' => 'b912ad97', - 'rsrc/css/layout/phabricator-source-code-view.css' => 'c6fc6834', + 'rsrc/css/layout/phabricator-source-code-view.css' => 'af54e277', 'rsrc/css/phui/button/phui-button-bar.css' => 'f1ff5494', 'rsrc/css/phui/button/phui-button-simple.css' => '8e1baf68', 'rsrc/css/phui/button/phui-button.css' => '1863cc6e', @@ -387,12 +386,11 @@ 'rsrc/js/application/diffusion/behavior-commit-graph.js' => '75b83cbb', 'rsrc/js/application/diffusion/behavior-diffusion-browse-file.js' => '054a0f0b', 'rsrc/js/application/diffusion/behavior-jump-to.js' => '73d09eef', - 'rsrc/js/application/diffusion/behavior-load-blame.js' => '42126667', 'rsrc/js/application/diffusion/behavior-locate-file.js' => '6d3e1947', 'rsrc/js/application/diffusion/behavior-pull-lastmodified.js' => 'f01586dc', 'rsrc/js/application/doorkeeper/behavior-doorkeeper-tag.js' => '1db13e70', 'rsrc/js/application/drydock/drydock-live-operation-status.js' => '901935ef', - 'rsrc/js/application/files/behavior-document-engine.js' => '9108ee1a', + 'rsrc/js/application/files/behavior-document-engine.js' => 'ac52a3be', 'rsrc/js/application/files/behavior-icon-composer.js' => '8499b6ab', 'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888', 'rsrc/js/application/harbormaster/behavior-harbormaster-log.js' => '191b4909', @@ -543,7 +541,7 @@ 'conpherence-thread-manager' => '4d863052', 'conpherence-transaction-css' => '85129c68', 'd3' => 'a11a5ff2', - 'differential-changeset-view-css' => 'bf84345b', + 'differential-changeset-view-css' => 'db34a142', 'differential-core-view-css' => '5b7b8ff4', 'differential-revision-add-comment-css' => 'c47f8c40', 'differential-revision-comment-css' => '14b8565a', @@ -554,7 +552,6 @@ 'diffusion-icons-css' => '0c15255e', 'diffusion-readme-css' => '419dd5b6', 'diffusion-repository-css' => 'ee6f20ec', - 'diffusion-source-css' => '5f35a3bd', 'diviner-shared-css' => '896f1d43', 'font-fontawesome' => 'e838e088', 'font-lato' => 'c7ccd872', @@ -607,7 +604,7 @@ 'javelin-behavior-diffusion-jump-to' => '73d09eef', 'javelin-behavior-diffusion-locate-file' => '6d3e1947', 'javelin-behavior-diffusion-pull-lastmodified' => 'f01586dc', - 'javelin-behavior-document-engine' => '9108ee1a', + 'javelin-behavior-document-engine' => 'ac52a3be', 'javelin-behavior-doorkeeper-tag' => '1db13e70', 'javelin-behavior-drydock-live-operation-status' => '901935ef', 'javelin-behavior-durable-column' => '2ae077e1', @@ -624,7 +621,6 @@ 'javelin-behavior-launch-icon-composer' => '48086888', 'javelin-behavior-lightbox-attachments' => '6b31879a', 'javelin-behavior-line-chart' => 'e4232876', - 'javelin-behavior-load-blame' => '42126667', 'javelin-behavior-maniphest-batch-selector' => 'ad54037e', 'javelin-behavior-maniphest-list-editor' => 'a9f88de2', 'javelin-behavior-maniphest-subpriority-editor' => '71237763', @@ -784,7 +780,7 @@ 'phabricator-search-results-css' => '505dd8cf', 'phabricator-shaped-request' => '7cbe244b', 'phabricator-slowvote-css' => 'a94b7230', - 'phabricator-source-code-view-css' => 'c6fc6834', + 'phabricator-source-code-view-css' => 'af54e277', 'phabricator-standard-page-view' => '34ee718b', 'phabricator-textareautils' => '320810c8', 'phabricator-title' => '485aaa6c', @@ -1153,11 +1149,6 @@ 'phabricator-diff-changeset-list', 'phabricator-diff-changeset', ), - 42126667 => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-request', - ), '4250a34e' => array( 'javelin-behavior', 'javelin-dom', @@ -1632,11 +1623,6 @@ 'javelin-stratcom', 'javelin-vector', ), - '9108ee1a' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-stratcom', - ), '92b9ec77' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1772,6 +1758,11 @@ 'javelin-dom', 'javelin-typeahead-normalizer', ), + 'ac52a3be' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-stratcom', + ), 'acd29eee' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1902,9 +1893,6 @@ 'javelin-stratcom', 'javelin-dom', ), - 'bf84345b' => array( - 'phui-inline-comment-view-css', - ), 'bff6884b' => array( 'javelin-install', 'javelin-dom', @@ -2021,6 +2009,9 @@ 'javelin-util', 'phabricator-shaped-request', ), + 'db34a142' => array( + 'phui-inline-comment-view-css', + ), 'dca75c0e' => array( 'multirow-row-manager', 'javelin-install', @@ -2367,7 +2358,6 @@ 'javelin-behavior-aphront-drag-and-drop-textarea', 'javelin-behavior-phabricator-object-selector', 'javelin-behavior-repository-crossreference', - 'javelin-behavior-load-blame', 'javelin-behavior-differential-user-select', 'javelin-behavior-aphront-more', 'phabricator-diff-inline', diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -199,7 +199,6 @@ 'javelin-behavior-aphront-drag-and-drop-textarea', 'javelin-behavior-phabricator-object-selector', 'javelin-behavior-repository-crossreference', - 'javelin-behavior-load-blame', 'javelin-behavior-differential-user-select', 'javelin-behavior-aphront-more', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -632,6 +632,7 @@ 'DiffusionAuditorsAddSelfHeraldAction' => 'applications/diffusion/herald/DiffusionAuditorsAddSelfHeraldAction.php', 'DiffusionAuditorsHeraldAction' => 'applications/diffusion/herald/DiffusionAuditorsHeraldAction.php', 'DiffusionBlameConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionBlameConduitAPIMethod.php', + 'DiffusionBlameController' => 'applications/diffusion/controller/DiffusionBlameController.php', 'DiffusionBlameQuery' => 'applications/diffusion/query/blame/DiffusionBlameQuery.php', 'DiffusionBlockHeraldAction' => 'applications/diffusion/herald/DiffusionBlockHeraldAction.php', 'DiffusionBranchListView' => 'applications/diffusion/view/DiffusionBranchListView.php', @@ -5889,6 +5890,7 @@ 'DiffusionAuditorsAddSelfHeraldAction' => 'DiffusionAuditorsHeraldAction', 'DiffusionAuditorsHeraldAction' => 'HeraldAction', 'DiffusionBlameConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', + 'DiffusionBlameController' => 'DiffusionController', 'DiffusionBlameQuery' => 'DiffusionQuery', 'DiffusionBlockHeraldAction' => 'HeraldAction', 'DiffusionBranchListView' => 'DiffusionView', diff --git a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php --- a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php +++ b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php @@ -59,6 +59,8 @@ 'browse/(?P.*)' => 'DiffusionBrowseController', 'document/(?P.*)' => 'DiffusionDocumentController', + 'blame/(?P.*)' + => 'DiffusionBlameController', 'lastmodified/(?P.*)' => 'DiffusionLastModifiedController', 'diff/' => 'DiffusionDiffController', 'tags/(?P.*)' => 'DiffusionTagListController', diff --git a/src/applications/diffusion/controller/DiffusionBlameController.php b/src/applications/diffusion/controller/DiffusionBlameController.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/controller/DiffusionBlameController.php @@ -0,0 +1,254 @@ +loadDiffusionContext(); + if ($response) { + return $response; + } + + $viewer = $this->getViewer(); + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $blame = $this->loadBlame(); + + $identifiers = array_fuse($blame); + if ($identifiers) { + $commits = id(new DiffusionCommitQuery()) + ->setViewer($viewer) + ->withRepository($repository) + ->withIdentifiers($identifiers) + ->execute(); + $commits = mpull($commits, null, 'getCommitIdentifier'); + } else { + $commits = array(); + } + + $commit_map = mpull($commits, 'getCommitIdentifier', 'getPHID'); + + $revisions = array(); + $revision_map = array(); + if ($commits) { + $revision_ids = id(new DifferentialRevision()) + ->loadIDsByCommitPHIDs(array_keys($commit_map)); + if ($revision_ids) { + $revisions = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withIDs($revision_ids) + ->execute(); + $revisions = mpull($revisions, null, 'getID'); + } + + foreach ($revision_ids as $commit_phid => $revision_id) { + // If the viewer can't actually see this revision, skip it. + if (!isset($revisions[$revision_id])) { + continue; + } + $revision_map[$commit_map[$commit_phid]] = $revision_id; + } + } + + $base_href = (string)$drequest->generateURI( + array( + 'action' => 'browse', + 'stable' => true, + )); + + $skip_text = pht('Skip Past This Commit'); + $skip_icon = id(new PHUIIconView()) + ->setIcon('fa-backward'); + + Javelin::initBehavior('phabricator-tooltips'); + + $handle_phids = array(); + foreach ($commits as $commit) { + $author_phid = $commit->getAuthorPHID(); + if ($author_phid) { + $handle_phids[] = $author_phid; + } + } + + foreach ($revisions as $revision) { + $handle_phids[] = $revision->getAuthorPHID(); + } + + $handles = $viewer->loadHandles($handle_phids); + + $map = array(); + foreach ($identifiers as $identifier) { + $revision_id = idx($revision_map, $identifier); + if ($revision_id) { + $revision = idx($revisions, $revision_id); + } else { + $revision = null; + } + + $skip_href = $base_href.'?before='.$identifier; + + $skip_link = javelin_tag( + 'a', + array( + 'href' => $skip_href, + 'sigil' => 'has-tooltip', + 'meta' => array( + 'tip' => $skip_text, + 'align' => 'E', + 'size' => 300, + ), + ), + $skip_icon); + + $commit = $commits[$identifier]; + + $author_phid = $commit->getAuthorPHID(); + if (!$author_phid && $revision) { + $author_phid = $revision->getAuthorPHID(); + } + + if (!$author_phid) { + // This means we couldn't identify an author for the commit or the + // revision. We just render a blank for alignment. + $author_style = null; + $author_href = null; + $author_sigil = null; + $author_meta = null; + } else { + $author_src = $handles[$author_phid]->getImageURI(); + $author_style = 'background-image: url('.$author_src.');'; + $author_href = $handles[$author_phid]->getURI(); + $author_sigil = 'has-tooltip'; + $author_meta = array( + 'tip' => $handles[$author_phid]->getName(), + 'align' => 'E', + ); + } + + $author_link = javelin_tag( + $author_href ? 'a' : 'span', + array( + 'class' => 'phabricator-source-blame-author', + 'style' => $author_style, + 'href' => $author_href, + 'sigil' => $author_sigil, + 'meta' => $author_meta, + )); + + $commit_link = javelin_tag( + 'a', + array( + 'href' => $commit->getURI(), + 'sigil' => 'has-tooltip', + 'meta' => array( + 'tip' => $this->renderCommitTooltip($commit, $handles), + 'align' => 'E', + 'size' => 600, + ), + ), + $commit->getLocalName()); + + $info = array( + $author_link, + $commit_link, + ); + + if ($revision) { + $revision_link = phutil_tag( + 'a', + array( + 'href' => $revision->getURI(), + 'sigil' => 'has-tooltip', + 'meta' => array( + 'tip' => $this->renderRevisionTooltip($revision, $handles), + 'align' => 'E', + 'size' => 600, + ), + ), + $revision->getMonogram()); + + + $info = array( + $info, + ' / ', + $revision_link, + ); + } + + $data = array( + 'skip' => $skip_link, + 'info' => hsprintf('%s', $info), + ); + + $map[$identifier] = $data; + } + + return id(new AphrontAjaxResponse())->setContent( + array( + 'blame' => $blame, + 'map' => $map, + )); + } + + private function loadBlame() { + $drequest = $this->getDiffusionRequest(); + + $commit = $drequest->getCommit(); + $path = $drequest->getPath(); + + $blame_timeout = 15; + + $blame = $this->callConduitWithDiffusionRequest( + 'diffusion.blame', + array( + 'commit' => $commit, + 'paths' => array($path), + 'timeout' => $blame_timeout, + )); + + return idx($blame, $path, array()); + } + + private function renderRevisionTooltip( + DifferentialRevision $revision, + $handles) { + $viewer = $this->getViewer(); + + $date = phabricator_date($revision->getDateModified(), $viewer); + $monogram = $revision->getMonogram(); + $title = $revision->getTitle(); + $header = "{$monogram} {$title}"; + + $author = $handles[$revision->getAuthorPHID()]->getName(); + + return "{$header}\n{$date} \xC2\xB7 {$author}"; + } + + private function renderCommitTooltip( + PhabricatorRepositoryCommit $commit, + $handles) { + + $viewer = $this->getViewer(); + + $date = phabricator_date($commit->getEpoch(), $viewer); + $summary = trim($commit->getSummary()); + + $author_phid = $commit->getAuthorPHID(); + if ($author_phid && isset($handles[$author_phid])) { + $author_name = $handles[$author_phid]->getName(); + } else { + $author_name = null; + } + + if ($author_name) { + return "{$summary}\n{$date} \xC2\xB7 {$author_name}"; + } else { + return "{$summary}\n{$date}"; + } + } + +} diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -110,12 +110,6 @@ } $path = $drequest->getPath(); - - // We need the blame information if blame is on and this is an Ajax request. - // If blame is on and this is a colorized request, we don't show blame at - // first (we ajax it in afterward) so we don't need to query for it. - $needs_blame = $request->isAjax(); - $params = array( 'commit' => $drequest->getCommit(), 'path' => $drequest->getPath(), @@ -184,11 +178,10 @@ $file->setName($basename); return $file->getRedirectResponse(); - } else { - $corpus = $this->buildGitLFSCorpus($lfs_ref); } + + $corpus = $this->buildGitLFSCorpus($lfs_ref); } else { - $this->loadLintMessages(); $this->coverage = $drequest->loadCoverage(); $show_editor = true; @@ -205,12 +198,6 @@ } } - if ($request->isAjax()) { - return id(new AphrontAjaxResponse())->setContent($corpus); - } - - require_celerity_resource('diffusion-source-css'); - $bar = $this->buildButtonBar($drequest, $show_editor); $header = $this->buildHeaderView($drequest); $header->setHeaderIcon('fa-file-code-o'); @@ -480,35 +467,6 @@ return $view; } - private function loadLintMessages() { - $drequest = $this->getDiffusionRequest(); - $branch = $drequest->loadBranch(); - - if (!$branch || !$branch->getLintCommit()) { - return; - } - - $this->lintCommit = $branch->getLintCommit(); - - $conn = id(new PhabricatorRepository())->establishConnection('r'); - - $where = ''; - if ($drequest->getLint()) { - $where = qsprintf( - $conn, - 'AND code = %s', - $drequest->getLint()); - } - - $this->lintMessages = queryfx_all( - $conn, - 'SELECT * FROM %T WHERE branchID = %d %Q AND path = %s', - PhabricatorRepository::TABLE_LINTMESSAGE, - $branch->getID(), - $where, - '/'.$drequest->getPath()); - } - private function buildButtonBar( DiffusionRequest $drequest, $show_editor) { @@ -550,33 +508,6 @@ ->setColor(PHUIButtonView::GREY); } - $href = null; - $show_lint = true; - if ($this->getRequest()->getStr('lint') !== null) { - $lint_text = pht('Hide Lint'); - $href = $base_uri->alter('lint', null); - - } else if ($this->lintCommit === null) { - $show_lint = false; - } else { - $lint_text = pht('Show Lint'); - $href = $this->getDiffusionRequest()->generateURI(array( - 'action' => 'browse', - 'commit' => $this->lintCommit, - ))->alter('lint', ''); - } - - if ($show_lint) { - $buttons[] = - id(new PHUIButtonView()) - ->setTag('a') - ->setText($lint_text) - ->setHref($href) - ->setIcon('fa-exclamation-triangle') - ->setDisabled(!$href) - ->setColor(PHUIButtonView::GREY); - } - $bar = id(new PHUILeftRightView()) ->setLeft($buttons) ->addClass('diffusion-action-bar full-mobile-buttons'); @@ -705,40 +636,6 @@ ->setColor(PHUIButtonView::GREY); } - private function renderInlines( - array $inlines, - $has_coverage, - $engine) { - - $rows = array(); - foreach ($inlines as $inline) { - - // TODO: This should use modern scaffolding code. - - $inline_view = id(new PHUIDiffInlineCommentDetailView()) - ->setUser($this->getViewer()) - ->setMarkupEngine($engine) - ->setInlineComment($inline) - ->render(); - - $row = array_fill(0, 3, phutil_tag('th')); - - $row[] = phutil_tag('td', array(), $inline_view); - - if ($has_coverage) { - $row[] = phutil_tag( - 'td', - array( - 'class' => 'cov cov-I', - )); - } - - $rows[] = phutil_tag('tr', array('class' => 'inline'), $row); - } - - return $rows; - } - private function buildErrorCorpus($message) { $text = id(new PHUIBoxView()) ->addPadding(PHUI::PADDING_LARGE) @@ -898,33 +795,6 @@ return head($parents); } - private function renderRevisionTooltip( - DifferentialRevision $revision, - $handles) { - $viewer = $this->getRequest()->getUser(); - - $date = phabricator_date($revision->getDateModified(), $viewer); - $id = $revision->getID(); - $title = $revision->getTitle(); - $header = "D{$id} {$title}"; - - $author = $handles[$revision->getAuthorPHID()]->getName(); - - return "{$header}\n{$date} \xC2\xB7 {$author}"; - } - - private function renderCommitTooltip( - PhabricatorRepositoryCommit $commit, - $author) { - - $viewer = $this->getRequest()->getUser(); - - $date = phabricator_date($commit->getEpoch(), $viewer); - $summary = trim($commit->getSummary()); - - return "{$summary}\n{$date} \xC2\xB7 {$author}"; - } - protected function markupText($text) { $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine(); $engine->setConfig('viewer', $this->getRequest()->getUser()); @@ -1108,127 +978,6 @@ return $view; } - private function loadBlame($path, $commit, $timeout) { - $blame = $this->callConduitWithDiffusionRequest( - 'diffusion.blame', - array( - 'commit' => $commit, - 'paths' => array($path), - 'timeout' => $timeout, - )); - - $identifiers = idx($blame, $path, null); - - if ($identifiers) { - $viewer = $this->getViewer(); - $drequest = $this->getDiffusionRequest(); - $repository = $drequest->getRepository(); - - $commits = id(new DiffusionCommitQuery()) - ->setViewer($viewer) - ->withRepository($repository) - ->withIdentifiers($identifiers) - // TODO: We only fetch this to improve author display behavior, but - // shouldn't really need to? - ->needCommitData(true) - ->execute(); - $commits = mpull($commits, null, 'getCommitIdentifier'); - } else { - $commits = array(); - } - - return array($identifiers, $commits); - } - - private function renderAuthorLinks(array $authors, $handles) { - $links = array(); - - foreach ($authors as $phid) { - if (!strlen($phid)) { - // This means we couldn't identify an author for the commit or the - // revision. We just render a blank for alignment. - $style = null; - $href = null; - $sigil = null; - $meta = null; - } else { - $src = $handles[$phid]->getImageURI(); - $style = 'background-image: url('.$src.');'; - $href = $handles[$phid]->getURI(); - $sigil = 'has-tooltip'; - $meta = array( - 'tip' => $handles[$phid]->getName(), - 'align' => 'E', - ); - } - - $links[$phid] = javelin_tag( - $href ? 'a' : 'span', - array( - 'class' => 'diffusion-author-link', - 'style' => $style, - 'href' => $href, - 'sigil' => $sigil, - 'meta' => $meta, - )); - } - - return $links; - } - - private function renderCommitLinks(array $commits, $handles) { - $links = array(); - foreach ($commits as $identifier => $commit) { - $tooltip = $this->renderCommitTooltip( - $commit, - $commit->renderAuthorShortName($handles)); - - $commit_link = javelin_tag( - 'a', - array( - 'href' => $commit->getURI(), - 'sigil' => 'has-tooltip', - 'meta' => array( - 'tip' => $tooltip, - 'align' => 'E', - 'size' => 600, - ), - ), - $commit->getLocalName()); - - $links[$identifier] = $commit_link; - } - - return $links; - } - - private function renderRevisionLinks(array $revisions, $handles) { - $links = array(); - - foreach ($revisions as $revision) { - $revision_id = $revision->getID(); - - $tooltip = $this->renderRevisionTooltip($revision, $handles); - - $revision_link = javelin_tag( - 'a', - array( - 'href' => '/'.$revision->getMonogram(), - 'sigil' => 'has-tooltip', - 'meta' => array( - 'tip' => $tooltip, - 'align' => 'E', - 'size' => 600, - ), - ), - $revision->getMonogram()); - - $links[$revision_id] = $revision_link; - } - - return $links; - } - private function getGitLFSRef(PhabricatorRepository $repository, $data) { if (!$repository->canUseGitLFS()) { return null; @@ -1375,13 +1124,4 @@ ->setTable($history_table); } - private function getLineNumberBaseURI() { - $drequest = $this->getDiffusionRequest(); - - return (string)$drequest->generateURI( - array( - 'action' => 'browse', - 'stable' => true, - )); - } } diff --git a/src/applications/diffusion/document/DiffusionDocumentRenderingEngine.php b/src/applications/diffusion/document/DiffusionDocumentRenderingEngine.php --- a/src/applications/diffusion/document/DiffusionDocumentRenderingEngine.php +++ b/src/applications/diffusion/document/DiffusionDocumentRenderingEngine.php @@ -70,7 +70,17 @@ } protected function willRenderRef(PhabricatorDocumentRef $ref) { - $ref->setSymbolMetadata($this->getSymbolMetadata()); + $drequest = $this->getDiffusionRequest(); + + $blame_uri = (string)$drequest->generateURI( + array( + 'action' => 'blame', + 'stable' => true, + )); + + $ref + ->setSymbolMetadata($this->getSymbolMetadata()) + ->setBlameURI($blame_uri); } private function getSymbolMetadata() { diff --git a/src/applications/files/document/PhabricatorDocumentEngine.php b/src/applications/files/document/PhabricatorDocumentEngine.php --- a/src/applications/files/document/PhabricatorDocumentEngine.php +++ b/src/applications/files/document/PhabricatorDocumentEngine.php @@ -38,6 +38,10 @@ return false; } + public function canBlame(PhabricatorDocumentRef $ref) { + return false; + } + final public function setEncodingConfiguration($config) { $this->encodingConfiguration = $config; return $this; diff --git a/src/applications/files/document/PhabricatorDocumentRef.php b/src/applications/files/document/PhabricatorDocumentRef.php --- a/src/applications/files/document/PhabricatorDocumentRef.php +++ b/src/applications/files/document/PhabricatorDocumentRef.php @@ -9,6 +9,7 @@ private $byteLength; private $snippet; private $symbolMetadata = array(); + private $blameURI; public function setFile(PhabricatorFile $file) { $this->file = $file; @@ -141,6 +142,13 @@ return $this->symbolMetadata; } + public function setBlameURI($blame_uri) { + $this->blameURI = $blame_uri; + return $this; + } + public function getBlameURI() { + return $this->blameURI; + } } diff --git a/src/applications/files/document/PhabricatorSourceDocumentEngine.php b/src/applications/files/document/PhabricatorSourceDocumentEngine.php --- a/src/applications/files/document/PhabricatorSourceDocumentEngine.php +++ b/src/applications/files/document/PhabricatorSourceDocumentEngine.php @@ -13,6 +13,10 @@ return true; } + public function canBlame(PhabricatorDocumentRef $ref) { + return true; + } + protected function getDocumentIconIcon(PhabricatorDocumentRef $ref) { return 'fa-code'; } @@ -45,9 +49,17 @@ } } + $options = array(); + if ($ref->getBlameURI()) { + $content = phutil_split_lines($content); + $blame = range(1, count($content)); + $blame = array_fuse($blame); + $options['blame'] = $blame; + } + return array( $messages, - $this->newTextDocumentContent($ref, $content), + $this->newTextDocumentContent($ref, $content, $options), ); } diff --git a/src/applications/files/document/PhabricatorTextDocumentEngine.php b/src/applications/files/document/PhabricatorTextDocumentEngine.php --- a/src/applications/files/document/PhabricatorTextDocumentEngine.php +++ b/src/applications/files/document/PhabricatorTextDocumentEngine.php @@ -15,14 +15,31 @@ protected function newTextDocumentContent( PhabricatorDocumentRef $ref, - $content) { - $lines = phutil_split_lines($content); + $content, + array $options = array()) { + + PhutilTypeSpec::checkMap( + $options, + array( + 'blame' => 'optional wild', + )); + + if (is_array($content)) { + $lines = $content; + } else { + $lines = phutil_split_lines($content); + } $view = id(new PhabricatorSourceCodeView()) ->setHighlights($this->getHighlightedLines()) ->setLines($lines) ->setSymbolMetadata($ref->getSymbolMetadata()); + $blame = idx($options, 'blame'); + if ($blame !== null) { + $view->setBlameMap($blame); + } + $message = null; if ($this->encodingMessage !== null) { $message = $this->newMessage($this->encodingMessage); diff --git a/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php b/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php --- a/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php +++ b/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php @@ -91,7 +91,8 @@ 'viewURI' => $view_uri, 'loadingMarkup' => hsprintf('%s', $loading), 'canEncode' => $candidate_engine->canConfigureEncoding($ref), - 'canHighlight' => $candidate_engine->CanConfigureHighlighting($ref), + 'canHighlight' => $candidate_engine->canConfigureHighlighting($ref), + 'canBlame' => $candidate_engine->canBlame($ref), ); } @@ -99,15 +100,20 @@ $control_id = celerity_generate_unique_node_id(); $icon = $engine->newDocumentIcon($ref); + $config = array( + 'controlID' => $control_id, + ); + if ($engine->shouldRenderAsync($ref)) { $content = $engine->newLoadingContent($ref); - $config = array( - 'renderControlID' => $control_id, - ); + $config['next'] = 'render'; } else { $this->willRenderRef($ref); $content = $engine->newDocument($ref); - $config = array(); + + if ($engine->canBlame($ref)) { + $config['next'] = 'blame'; + } } Javelin::initBehavior('document-engine', $config); @@ -135,6 +141,10 @@ 'uri' => '/services/highlight/', 'value' => $highlight_setting, ), + 'blame' => array( + 'uri' => $ref->getBlameURI(), + 'value' => null, + ), ); $view_button = id(new PHUIButtonView()) diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -703,6 +703,7 @@ case 'history': case 'graph': case 'clone': + case 'blame': case 'browse': case 'document': case 'change': @@ -782,6 +783,7 @@ case 'change': case 'history': case 'graph': + case 'blame': case 'browse': case 'document': case 'lastmodified': diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -416,28 +416,6 @@ return $repository->formatCommitName($identifier, $local = true); } - public function renderAuthorLink($handles) { - $author_phid = $this->getAuthorPHID(); - if ($author_phid && isset($handles[$author_phid])) { - return $handles[$author_phid]->renderLink(); - } - - return $this->renderAuthorShortName($handles); - } - - public function renderAuthorShortName($handles) { - $author_phid = $this->getAuthorPHID(); - if ($author_phid && isset($handles[$author_phid])) { - return $handles[$author_phid]->getName(); - } - - $data = $this->getCommitData(); - $name = $data->getAuthorName(); - - $parsed = new PhutilEmailAddress($name); - return nonempty($parsed->getDisplayName(), $parsed->getAddress()); - } - /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/view/layout/PhabricatorSourceCodeView.php b/src/view/layout/PhabricatorSourceCodeView.php --- a/src/view/layout/PhabricatorSourceCodeView.php +++ b/src/view/layout/PhabricatorSourceCodeView.php @@ -9,6 +9,7 @@ private $truncatedFirstBytes = false; private $truncatedFirstLines = false; private $symbolMetadata; + private $blameMap; public function setLines(array $lines) { $this->lines = $lines; @@ -49,7 +50,19 @@ return $this->symbolMetadata; } + public function setBlameMap(array $map) { + $this->blameMap = $map; + return $this; + } + + public function getBlameMap() { + return $this->blameMap; + } + public function render() { + $blame_map = $this->getBlameMap(); + $has_blame = ($blame_map !== null); + require_celerity_resource('phabricator-source-code-view-css'); require_celerity_resource('syntax-highlighting-css'); @@ -85,7 +98,6 @@ $base_uri = (string)$this->uri; foreach ($lines as $line) { - // NOTE: See phabricator-oncopy behavior. $content_line = hsprintf("\xE2\x80\x8B%s", $line); @@ -114,10 +126,40 @@ $line_number); } + if ($has_blame) { + $lines = idx($blame_map, $line_number); + + if ($lines) { + $skip_blame = 'skip;'.$lines; + $info_blame = 'info;'.$lines; + } else { + $skip_blame = null; + $info_blame = null; + } + + $blame_cells = array( + phutil_tag( + 'th', + array( + 'class' => 'phabricator-source-blame-skip', + 'data-blame' => $skip_blame, + )), + phutil_tag( + 'th', + array( + 'class' => 'phabricator-source-blame-info', + 'data-blame' => $info_blame, + )), + ); + } else { + $blame_cells = null; + } + $rows[] = phutil_tag( 'tr', $row_attributes, array( + $blame_cells, phutil_tag( 'th', array( diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -165,10 +165,6 @@ padding: 0; } -.diffusion-source td.cov { - padding: 0 8px; -} - td.cov-U { background: #dd8866; } diff --git a/webroot/rsrc/css/application/diffusion/diffusion-source.css b/webroot/rsrc/css/application/diffusion/diffusion-source.css deleted file mode 100644 --- a/webroot/rsrc/css/application/diffusion/diffusion-source.css +++ /dev/null @@ -1,102 +0,0 @@ -/** - * @provides diffusion-source-css - */ - -.diffusion-source { - width: 100%; - background: {$page.content}; - overflow: hidden; -} - -.device-phone .diffusion-source-wrap { - overflow: scroll; - -webkit-overflow-scrolling: touch; -} - -.diffusion-source tr.phabricator-source-highlight { - background: {$sh-yellowbackground}; -} - -.diffusion-source th { - text-align: right; - vertical-align: top; - background: {$lightgreybackground}; - color: {$bluetext}; - border-right: 1px solid {$thinblueborder}; -} - -.diffusion-source td { - vertical-align: top; - white-space: pre-wrap; - padding-top: 1px; - padding-bottom: 1px; - padding-left: 8px; - width: 100%; - word-break: break-all; -} - -.device .diffusion-source td { - word-break: normal; - white-space: nowrap; -} - -.diffusion-browse-type-form { - float: right; -} - -.diffusion-blame-link, -.diffusion-rev-link { - white-space: nowrap; -} - -.diffusion-blame-link { - min-width: 28px; -} - -.diffusion-source th.diffusion-rev-link { - text-align: left; - min-width: 130px; -} - -.diffusion-blame-link a, -.diffusion-rev-link a, -.diffusion-line-link a { - color: {$darkbluetext}; -} - -.diffusion-rev-link a { - margin: 0 8px 0 0; - display: inline-block; -} - -.diffusion-rev-link span { - display: inline-block; - margin-right: 4px; - margin-left: -4px; - color: {$lightgreytext}; -} - -.diffusion-blame-link a, -.diffusion-line-link a { - /* Give the user a larger click target. */ - display: block; - padding: 2px 8px; -} - -.diffusion-line-link { - -moz-user-select: -moz-none; - -khtml-user-select: none; - -webkit-user-select: none; - -ms-user-select: none; - user-select: none; -} - -.diffusion-rev-link .diffusion-author-link { - display: inline-block; - padding: 0; - margin: 2px 6px -4px 8px; - width: 16px; - height: 16px; - background-size: 100% 100%; - background-repeat: no-repeat; -} diff --git a/webroot/rsrc/css/layout/phabricator-source-code-view.css b/webroot/rsrc/css/layout/phabricator-source-code-view.css --- a/webroot/rsrc/css/layout/phabricator-source-code-view.css +++ b/webroot/rsrc/css/layout/phabricator-source-code-view.css @@ -68,3 +68,46 @@ .phabricator-source-code-summary .phabricator-source-code { white-space: nowrap; } + + +.phabricator-source-blame-skip, +.phabricator-source-blame-info { + -moz-user-select: -moz-none; + -khtml-user-select: none; + -webkit-user-select: none; + -ms-user-select: none; + user-select: none; +} + +.phabricator-source-blame-skip { + min-width: 28px; + border-right: 1px solid {$thinblueborder}; +} + +.phabricator-source-blame-info { + color: {$lightgreytext}; + white-space: nowrap; + min-width: 130px; + border-right: 1px solid {$paste.border}; + padding-right: 8px; +} + +.phabricator-source-blame-skip a { + /* Give the user a larger click target. */ + display: block; + padding: 2px 8px; +} + +.device-desktop .phabricator-source-blame-skip a:hover { + background: {$bluebackground}; +} + +.phabricator-source-blame-author { + display: inline-block; + padding: 0; + margin: 2px 6px -4px 8px; + width: 16px; + height: 16px; + background-size: 100% 100%; + background-repeat: no-repeat; +} diff --git a/webroot/rsrc/js/application/diffusion/behavior-load-blame.js b/webroot/rsrc/js/application/diffusion/behavior-load-blame.js deleted file mode 100644 --- a/webroot/rsrc/js/application/diffusion/behavior-load-blame.js +++ /dev/null @@ -1,14 +0,0 @@ -/** - * @provides javelin-behavior-load-blame - * @requires javelin-behavior - * javelin-dom - * javelin-request - */ - -JX.behavior('load-blame', function(config) { - - new JX.Request(location.href, function (response) { - JX.DOM.setContent(JX.$(config.id), JX.$H(response)); - }).send(); - -}); diff --git a/webroot/rsrc/js/application/files/behavior-document-engine.js b/webroot/rsrc/js/application/files/behavior-document-engine.js --- a/webroot/rsrc/js/application/files/behavior-document-engine.js +++ b/webroot/rsrc/js/application/files/behavior-document-engine.js @@ -7,8 +7,6 @@ JX.behavior('document-engine', function(config, statics) { - - function onmenu(e) { var node = e.getNode('document-engine-view-dropdown'); var data = JX.Stratcom.getData(node); @@ -213,15 +211,91 @@ JX.DOM.setContent(viewport, JX.$H(r.markup)); } + function blame(data) { + if (!data.blame.uri) { + return; + } + + if (!data.blame.value) { + new JX.Request(data.blame.uri, JX.bind(null, onblame, data)) + .send(); + return; + } + + var viewport = JX.$(data.viewportID); + + var cells = JX.DOM.scry(viewport, 'th'); + + for (var ii = 0; ii < cells.length; ii++) { + var cell = cells[ii]; + + var spec = cell.getAttribute('data-blame'); + if (!spec) { + continue; + } + + spec = spec.split(';'); + var type = spec[0]; + var lines = spec[1]; + + var content = null; + switch (type) { + case 'skip': + content = renderSkip(data.blame.value, lines); + break; + case 'info': + content = renderInfo(data.blame.value, lines); + break; + } + + JX.DOM.setContent(cell, content); + } + } + + function onblame(data, r) { + data.blame.value = r; + blame(data); + } + + function renderSkip(blame, lines) { + var commit = blame.blame[lines - 1]; + if (!commit) { + return null; + } + + var spec = blame.map[commit]; + + return JX.$H(spec.skip); + } + + function renderInfo(blame, lines) { + var commit = blame.blame[lines - 1]; + if (!commit) { + return null; + } + + var spec = blame.map[commit]; + + return JX.$H(spec.info); + } + if (!statics.initialized) { JX.Stratcom.listen('click', 'document-engine-view-dropdown', onmenu); statics.initialized = true; } - if (config && config.renderControlID) { - var control = JX.$(config.renderControlID); + if (config && config.controlID) { + var control = JX.$(config.controlID); var data = JX.Stratcom.getData(control); - onview(data, null, true); + + switch (config.next) { + case 'render': + onview(data, null, true); + break; + case 'blame': + blame(data); + break; + } } });