diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 07fe9c24c4..a48e51ad0b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -224,6 +224,7 @@ phutil_register_library_map(array( 'DifferentialChangeType' => 'applications/differential/constants/DifferentialChangeType.php', 'DifferentialChangeset' => 'applications/differential/storage/DifferentialChangeset.php', 'DifferentialChangesetDetailView' => 'applications/differential/view/DifferentialChangesetDetailView.php', + 'DifferentialChangesetFileTreeSideNavBuilder' => 'applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php', 'DifferentialChangesetListView' => 'applications/differential/view/DifferentialChangesetListView.php', 'DifferentialChangesetParser' => 'applications/differential/parser/DifferentialChangesetParser.php', 'DifferentialChangesetParserTestCase' => 'applications/differential/parser/__tests__/DifferentialChangesetParserTestCase.php', @@ -2570,12 +2571,13 @@ phutil_register_library_map(array( 1 => 'PhabricatorMarkupInterface', 2 => 'PonderVotableInterface', 3 => 'PhabricatorSubscribableInterface', + 4 => 'PhabricatorPolicyInterface', ), 'PonderQuestionAskController' => 'PonderController', 'PonderQuestionDetailView' => 'AphrontView', 'PonderQuestionEditor' => 'PhabricatorEditor', 'PonderQuestionPreviewController' => 'PonderController', - 'PonderQuestionQuery' => 'PhabricatorCursorPagedPolicyQuery', + 'PonderQuestionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PonderQuestionSummaryView' => 'AphrontView', 'PonderQuestionViewController' => 'PonderController', 'PonderReplyHandler' => 'PhabricatorMailReplyHandler', diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 72a6e9f2ee..54498cb9c0 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -398,12 +398,17 @@ final class DifferentialRevisionViewController extends DifferentialController { PhabricatorFeedStoryNotification::updateObjectNotificationViews( $user, $revision->getPHID()); + $object_id = 'D'.$revision->getID(); + $top_anchor = id(new PhabricatorAnchorView()) ->setAnchorName('top') ->setNavigationMarker(true); - $nav = $this->buildSideNavView($revision, $changesets); - $nav->selectFilter(''); + $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) + ->setAnchorName('top') + ->setTitle('D'.$revision->getID()) + ->setBaseURI(new PhutilURI('/D'.$revision->getID())) + ->build($changesets); $nav->appendChild( array( $reviewer_warning, @@ -412,7 +417,6 @@ final class DifferentialRevisionViewController extends DifferentialController { $page_pane, )); - $object_id = 'D'.$revision->getID(); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addCrumb( @@ -960,103 +964,4 @@ final class DifferentialRevisionViewController extends DifferentialController { return id(new AphrontRedirectResponse())->setURI($file->getBestURI()); } - - private function buildSideNavView( - DifferentialRevision $revision, - array $changesets) { - - $nav = new AphrontSideNavFilterView(); - $nav->setBaseURI(new PhutilURI('/D'.$revision->getID())); - $nav->setFlexible(true); - - $nav->addFilter('top', 'D'.$revision->getID(), '#top'); - - $tree = new PhutilFileTree(); - foreach ($changesets as $changeset) { - try { - $tree->addPath($changeset->getFilename(), $changeset); - } catch (Exception $ex) { - // TODO: See T1702. When viewing the versus diff of diffs, we may - // have files with the same filename. For example, if you have a setup - // like this in SVN: - // - // a/ - // README - // b/ - // README - // - // ...and you run "arc diff" once from a/, and again from b/, you'll - // get two diffs with path README. However, in the versus diff view we - // will compute their absolute repository paths and detect that they - // aren't really the same file. This is correct, but causes us to - // throw when inserting them. - // - // We should probably compute the smallest unique path for each file - // and show these as "a/README" and "b/README" when diffed against - // one another. However, we get this wrong in a lot of places (the - // other TOC shows two "README" files, and we generate the same anchor - // hash for both) so I'm just stopping the bleeding until we can get - // a proper fix in place. - } - } - - require_celerity_resource('phabricator-filetree-view-css'); - - $filetree = array(); - - $path = $tree; - while (($path = $path->getNextNode())) { - $data = $path->getData(); - - $name = $path->getName(); - $style = 'padding-left: '.(2 + (3 * $path->getDepth())).'px'; - - $href = null; - if ($data) { - $href = '#'.$data->getAnchorName(); - $title = $name; - $icon = 'phabricator-filetree-icon-file'; - } else { - $name .= '/'; - $title = $path->getFullPath().'/'; - $icon = 'phabricator-filetree-icon-dir'; - } - - $icon = phutil_render_tag( - 'span', - array( - 'class' => 'phabricator-filetree-icon '.$icon, - ), - ''); - - $name_element = phutil_render_tag( - 'span', - array( - 'class' => 'phabricator-filetree-name', - ), - phutil_escape_html($name)); - - $filetree[] = javelin_render_tag( - $href ? 'a' : 'span', - array( - 'href' => $href, - 'style' => $style, - 'title' => $title, - 'class' => 'phabricator-filetree-item', - ), - $icon.$name_element); - } - $tree->destroy(); - - $filetree = - '
'. - implode("\n", $filetree). - '
'; - $nav->addFilter('toc', 'Table of Contents', '#toc'); - $nav->addCustomBlock($filetree); - $nav->addFilter('comment', 'Add Comment', '#comment'); - $nav->setActive(true); - - return $nav; - } } diff --git a/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php b/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php new file mode 100644 index 0000000000..f9d4fa1662 --- /dev/null +++ b/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php @@ -0,0 +1,134 @@ +anchorName = $anchor_name; + return $this; + } + public function getAnchorName() { + return $this->anchorName; + } + + public function setBaseURI(PhutilURI $base_uri) { + $this->baseURI = $base_uri; + return $this; + } + public function getBaseURI() { + return $this->baseURI; + } + + public function setTitle($title) { + $this->title = $title; + return $this; + } + public function getTitle() { + return $this->title; + } + + public function build(array $changesets) { + assert_instances_of($changesets, 'DifferentialChangeset'); + + $nav = new AphrontSideNavFilterView(); + $nav->setBaseURI($this->getBaseURI()); + $nav->setFlexible(true); + + $anchor = $this->getAnchorName(); + $nav->addFilter($anchor, $this->getTitle(), '#'.$anchor); + + $tree = new PhutilFileTree(); + foreach ($changesets as $changeset) { + try { + $tree->addPath($changeset->getFilename(), $changeset); + } catch (Exception $ex) { + // TODO: See T1702. When viewing the versus diff of diffs, we may + // have files with the same filename. For example, if you have a setup + // like this in SVN: + // + // a/ + // README + // b/ + // README + // + // ...and you run "arc diff" once from a/, and again from b/, you'll + // get two diffs with path README. However, in the versus diff view we + // will compute their absolute repository paths and detect that they + // aren't really the same file. This is correct, but causes us to + // throw when inserting them. + // + // We should probably compute the smallest unique path for each file + // and show these as "a/README" and "b/README" when diffed against + // one another. However, we get this wrong in a lot of places (the + // other TOC shows two "README" files, and we generate the same anchor + // hash for both) so I'm just stopping the bleeding until we can get + // a proper fix in place. + } + } + + require_celerity_resource('phabricator-filetree-view-css'); + + $filetree = array(); + + $path = $tree; + while (($path = $path->getNextNode())) { + $data = $path->getData(); + + $name = $path->getName(); + $style = 'padding-left: '.(2 + (3 * $path->getDepth())).'px'; + + $href = null; + if ($data) { + $href = '#'.$data->getAnchorName(); + $title = $name; + $icon = 'phabricator-filetree-icon-file'; + } else { + $name .= '/'; + $title = $path->getFullPath().'/'; + $icon = 'phabricator-filetree-icon-dir'; + } + + $icon = phutil_render_tag( + 'span', + array( + 'class' => 'phabricator-filetree-icon '.$icon, + ), + ''); + + $name_element = phutil_render_tag( + 'span', + array( + 'class' => 'phabricator-filetree-name', + ), + phutil_escape_html($name)); + + $filetree[] = javelin_render_tag( + $href ? 'a' : 'span', + array( + 'href' => $href, + 'style' => $style, + 'title' => $title, + 'class' => 'phabricator-filetree-item', + ), + $icon.$name_element); + } + $tree->destroy(); + + $filetree = + '
'. + implode("\n", $filetree). + '
'; + $nav->addFilter('toc', 'Table of Contents', '#toc'); + $nav->addCustomBlock($filetree); + $nav->addFilter('comment', 'Add Comment', '#comment'); + $nav->setActive(true); + $nav->selectFilter(''); + + return $nav; + } + +} + diff --git a/src/applications/differential/view/DifferentialRevisionDetailView.php b/src/applications/differential/view/DifferentialRevisionDetailView.php index 5e9b966ebe..ee3e6f7c3b 100644 --- a/src/applications/differential/view/DifferentialRevisionDetailView.php +++ b/src/applications/differential/view/DifferentialRevisionDetailView.php @@ -83,7 +83,7 @@ final class DifferentialRevisionDetailView extends AphrontView { } } if ($next_step) { - $properties->addProperty('Next Step:', $next_step); + $properties->addProperty('Next Step', $next_step); } foreach ($this->auxiliaryFields as $field) { diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index a1d1e24b78..9ed36ea8c0 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -10,13 +10,6 @@ final class DiffusionBrowseController extends DiffusionController { $content = array(); - $content[] = $this->buildCrumbs( - array( - 'branch' => true, - 'path' => true, - 'view' => 'browse', - )); - if ($drequest->getTagContent()) { $title = 'Tag: '.$drequest->getSymbolicCommit(); @@ -34,6 +27,7 @@ final class DiffusionBrowseController extends DiffusionController { DiffusionBrowseQuery::REASON_IS_FILE) { $controller = new DiffusionBrowseFileController($this->getRequest()); $controller->setDiffusionRequest($drequest); + $controller->setCurrentApplication($this->getCurrentApplication()); return $this->delegateToController($controller); } @@ -85,7 +79,15 @@ final class DiffusionBrowseController extends DiffusionController { $nav = $this->buildSideNav('browse', false); $nav->appendChild($content); - return $this->buildStandardPageResponse( + $crumbs = $this->buildCrumbs( + array( + 'branch' => true, + 'path' => true, + 'view' => 'browse', + )); + $nav->setCrumbs($crumbs); + + return $this->buildApplicationPage( $nav, array( 'title' => array( diff --git a/src/applications/diffusion/controller/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/DiffusionBrowseFileController.php index a62b62d7b0..3729ce9c60 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseFileController.php @@ -69,12 +69,6 @@ final class DiffusionBrowseFileController extends DiffusionController { // Render the page. $content = array(); - $content[] = $this->buildCrumbs( - array( - 'branch' => true, - 'path' => true, - 'view' => 'browse', - )); $follow = $request->getStr('follow'); if ($follow) { @@ -114,10 +108,17 @@ final class DiffusionBrowseFileController extends DiffusionController { $nav = $this->buildSideNav('browse', true); $nav->appendChild($content); + $crumbs = $this->buildCrumbs( + array( + 'branch' => true, + 'path' => true, + 'view' => 'browse', + )); + $nav->setCrumbs($crumbs); $basename = basename($this->getDiffusionRequest()->getPath()); - return $this->buildStandardPageResponse( + return $this->buildApplicationPage( $nav, array( 'title' => $basename, diff --git a/src/applications/diffusion/controller/DiffusionChangeController.php b/src/applications/diffusion/controller/DiffusionChangeController.php index f09786769d..cabe90feb0 100644 --- a/src/applications/diffusion/controller/DiffusionChangeController.php +++ b/src/applications/diffusion/controller/DiffusionChangeController.php @@ -45,13 +45,6 @@ final class DiffusionChangeController extends DiffusionController { DifferentialChangesetParser::WHITESPACE_SHOW_ALL); $changeset_view->setUser($this->getRequest()->getUser()); - $content[] = $this->buildCrumbs( - array( - 'branch' => true, - 'path' => true, - 'view' => 'change', - )); - // TODO: This is pretty awkward, unify the CSS between Diffusion and // Differential better. require_celerity_resource('differential-core-view-css'); @@ -62,8 +55,15 @@ final class DiffusionChangeController extends DiffusionController { $nav = $this->buildSideNav('change', true); $nav->appendChild($content); + $crumbs = $this->buildCrumbs( + array( + 'branch' => true, + 'path' => true, + 'view' => 'change', + )); + $nav->setCrumbs($crumbs); - return $this->buildStandardPageResponse( + return $this->buildApplicationPage( $nav, array( 'title' => 'Change', diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 0d654a29e7..1d91f059d9 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -26,10 +26,6 @@ final class DiffusionCommitController extends DiffusionController { $callsign = $drequest->getRepository()->getCallsign(); $content = array(); - $content[] = $this->buildCrumbs(array( - 'commit' => true, - )); - $repository = $drequest->getRepository(); $commit = $drequest->loadCommit(); @@ -51,6 +47,10 @@ final class DiffusionCommitController extends DiffusionController { $commit_data = $drequest->loadCommitData(); $commit->attachCommitData($commit_data); + $top_anchor = id(new PhabricatorAnchorView()) + ->setAnchorName('top') + ->setNavigationMarker(true); + $is_foreign = $commit_data->getCommitDetail('foreign-svn-stub'); if ($is_foreign) { $subpath = $commit_data->getCommitDetail('svn-subpath'); @@ -64,6 +64,7 @@ final class DiffusionCommitController extends DiffusionController { "didn't affect the tracked subdirectory ('". phutil_escape_html($subpath)."'), so no information is available."); $content[] = $error_panel; + $content[] = $top_anchor; } else { $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); @@ -73,22 +74,32 @@ final class DiffusionCommitController extends DiffusionController { $parent_query = DiffusionCommitParentsQuery::newFromDiffusionRequest( $drequest); - $headsup_panel = new AphrontHeadsupView(); - $headsup_panel->setHeader('Commit Detail'); - $headsup_panel->setActionList( - $this->renderHeadsupActionList($commit, $repository)); - $headsup_panel->setProperties( - $this->getCommitProperties( - $commit, - $commit_data, - $parent_query->loadParents())); + $headsup_view = id(new PhabricatorHeaderView()) + ->setHeader('Commit Detail'); - $headsup_panel->appendChild( + $headsup_actions = $this->renderHeadsupActionList($commit, $repository); + + $commit_properties = $this->loadCommitProperties( + $commit, + $commit_data, + $parent_query->loadParents() + ); + $property_list = id(new PhabricatorPropertyListView()) + ->setHasKeyboardShortcuts(true); + foreach ($commit_properties as $key => $value) { + $property_list->addProperty($key, $value); + } + + $property_list->addTextContent( '
'. - $engine->markupText($commit_data->getCommitMessage()). - '
'); + $engine->markupText($commit_data->getCommitMessage()). + '' + ); - $content[] = $headsup_panel; + $content[] = $top_anchor; + $content[] = $headsup_view; + $content[] = $headsup_actions; + $content[] = $property_list; } $query = new PhabricatorAuditQuery(); @@ -289,19 +300,43 @@ final class DiffusionCommitController extends DiffusionController { 'id' => $pane_id ), $change_list->render(). + id(new PhabricatorAnchorView()) + ->setAnchorName('comment') + ->setNavigationMarker(true) + ->render(). $add_comment_view); $content[] = $main_pane; } - return $this->buildStandardPageResponse( - $content, + $commit_id = 'r'.$callsign.$commit->getCommitIdentifier(); + $short_name = DiffusionView::nameCommit( + $repository, + $commit->getCommitIdentifier() + ); + $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) + ->setAnchorName('top') + ->setTitle($short_name) + ->setBaseURI(new PhutilURI('/'.$commit_id)) + ->build($changesets); + foreach ($content as $child) { + $nav->appendChild($child); + } + + $crumbs = $this->buildCrumbs(array( + 'commit' => true, + )); + $nav->setCrumbs($crumbs); + + return $this->buildApplicationPage( + $nav, array( - 'title' => 'r'.$callsign.$commit->getCommitIdentifier(), - )); + 'title' => $commit_id + ) + ); } - private function getCommitProperties( + private function loadCommitProperties( PhabricatorRepositoryCommit $commit, PhabricatorRepositoryCommitData $data, array $parents) { @@ -766,75 +801,54 @@ final class DiffusionCommitController extends DiffusionController { $request = $this->getRequest(); $user = $request->getUser(); - $actions = array(); + $actions = id(new PhabricatorActionListView()) + ->setUser($user) + ->setObject($commit); // TODO -- integrate permissions into whether or not this action is shown $uri = '/diffusion/'.$repository->getCallSign().'/commit/'. $commit->getCommitIdentifier().'/edit/'; - $action = new AphrontHeadsupActionView(); - $action->setClass('action-edit'); - $action->setURI($uri); - $action->setName('Edit Commit'); - $action->setWorkflow(false); - $actions[] = $action; - require_celerity_resource('phabricator-flag-css'); - $flag = PhabricatorFlagQuery::loadUserFlag($user, $commit->getPHID()); - if ($flag) { - $class = PhabricatorFlagColor::getCSSClass($flag->getColor()); - $color = PhabricatorFlagColor::getColorName($flag->getColor()); - - $action = new AphrontHeadsupActionView(); - $action->setClass('flag-clear '.$class); - $action->setURI('/flag/delete/'.$flag->getID().'/'); - $action->setName('Remove '.$color.' Flag'); - $action->setWorkflow(true); - $actions[] = $action; - } else { - $action = new AphrontHeadsupActionView(); - $action->setClass('phabricator-flag-ghost'); - $action->setURI('/flag/edit/'.$commit->getPHID().'/'); - $action->setName('Flag Commit'); - $action->setWorkflow(true); - $actions[] = $action; - } + $action = id(new PhabricatorActionView()) + ->setName('Edit Commit') + ->setHref($uri) + ->setIcon('edit'); + $actions->addAction($action); require_celerity_resource('phabricator-object-selector-css'); require_celerity_resource('javelin-behavior-phabricator-object-selector'); if (PhabricatorEnv::getEnvConfig('maniphest.enabled')) { - $action = new AphrontHeadsupActionView(); - $action->setName('Edit Maniphest Tasks'); - $action->setURI('/search/attach/'.$commit->getPHID().'/TASK/edge/'); - $action->setWorkflow(true); - $action->setClass('attach-maniphest'); - $actions[] = $action; + $action = id(new PhabricatorActionView()) + ->setName('Edit Maniphest Tasks') + ->setIcon('attach') + ->setHref('/search/attach/'.$commit->getPHID().'/TASK/edge/') + ->setWorkflow(true); + $actions->addAction($action); } if ($user->getIsAdmin()) { - $action = new AphrontHeadsupActionView(); - $action->setName('MetaMTA Transcripts'); - $action->setURI('/mail/?phid='.$commit->getPHID()); - $action->setClass('transcripts-metamta'); - $actions[] = $action; + $action = id(new PhabricatorActionView()) + ->setName('MetaMTA Transcripts') + ->setIcon('file') + ->setHref('/mail/?phid='.$commit->getPHID()); + $actions->addAction($action); } - $action = new AphrontHeadsupActionView(); - $action->setName('Herald Transcripts'); - $action->setURI('/herald/transcript/?phid='.$commit->getPHID()); - $action->setClass('transcripts-herald'); - $actions[] = $action; + $action = id(new PhabricatorActionView()) + ->setName('Herald Transcripts') + ->setIcon('file') + ->setHref('/herald/transcript/?phid='.$commit->getPHID()) + ->setWorkflow(true); + $actions->addAction($action); - $action = new AphrontHeadsupActionView(); - $action->setName('Download Raw Diff'); - $action->setURI($request->getRequestURI()->alter('diff', true)); - $action->setClass('action-download'); - $actions[] = $action; + $action = id(new PhabricatorActionView()) + ->setName('Download Raw Diff') + ->setHref($request->getRequestURI()->alter('diff', true)) + ->setIcon('download'); + $actions->addAction($action); - $action_list = new AphrontHeadsupActionListView(); - $action_list->setActions($actions); - - return $action_list; + return $actions; } private function buildRefs(DiffusionRequest $request) { diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php index 2c16ab2246..08fffae287 100644 --- a/src/applications/diffusion/controller/DiffusionController.php +++ b/src/applications/diffusion/controller/DiffusionController.php @@ -72,7 +72,7 @@ abstract class DiffusionController extends PhabricatorController { $selected_href = $href; } - $nav->addFilter($href, $name); + $nav->addFilter($href, $name, $href); } $nav->selectFilter($selected_href, null); @@ -88,9 +88,11 @@ abstract class DiffusionController extends PhabricatorController { } public function buildCrumbs(array $spec = array()) { - $crumbs = new AphrontCrumbsView(); + $crumbs = $this->buildApplicationCrumbs(); $crumb_list = $this->buildCrumbList($spec); - $crumbs->setCrumbs($crumb_list); + foreach ($crumb_list as $crumb) { + $crumbs->addCrumb($crumb); + } return $crumbs; } @@ -155,15 +157,11 @@ abstract class DiffusionController extends PhabricatorController { $repository = null; } - if ($repository) { - $crumb_list[] = phutil_render_tag( - 'a', - array( - 'href' => '/diffusion/', - ), - 'Diffusion'); - } else { - $crumb_list[] = 'Diffusion'; + $crumb = id(new PhabricatorCrumbView()) + ->setName('Diffusion') + ->setHref('/diffusion/'); + $crumb_list[] = $crumb; + if (!$repository) { return $crumb_list; } @@ -177,49 +175,53 @@ abstract class DiffusionController extends PhabricatorController { } } - if (!$spec['view'] && !$spec['commit'] - && !$spec['tags'] && !$spec['branches']) { - $crumb_list[] = $repository_name; - return $crumb_list; + $crumb = id(new PhabricatorCrumbView()) + ->setName($repository_name); + if (!$spec['view'] && !$spec['commit'] && + !$spec['tags'] && !$spec['branches']) { + $crumb_list[] = $crumb; + return $crumb_list; } - - $crumb_list[] = phutil_render_tag( - 'a', - array( - 'href' => "/diffusion/{$callsign}/", - ), - $repository_name); + $crumb->setHref("/diffusion/{$callsign}/"); + $crumb_list[] = $crumb; $raw_commit = $drequest->getRawCommit(); if ($spec['tags']) { + $crumb = new PhabricatorCrumbView(); if ($spec['commit']) { - $crumb_list[] = "Tags for ".phutil_render_tag( - 'a', + $crumb->setName( + "Tags for r{$callsign}{$raw_commit}" + ); + $crumb->setHref($drequest->generateURI( array( - 'href' => $drequest->generateURI( - array( - 'action' => 'commit', - 'commit' => $raw_commit, - )), - ), - phutil_escape_html("r{$callsign}{$raw_commit}")); + 'action' => 'commit', + 'commit' => $raw_commit, + )) + ); } else { - $crumb_list[] = 'Tags'; + $crumb->setName('Tags'); } + $crumb_list[] = $crumb; return $crumb_list; } if ($spec['branches']) { - $crumb_list[] = 'Branches'; + $crumb = id(new PhabricatorCrumbView()) + ->setName('Branches'); + $crumb_list[] = $crumb; return $crumb_list; } if ($spec['commit']) { - $crumb_list[] = "r{$callsign}{$raw_commit}"; + $crumb = id(new PhabricatorCrumbView()) + ->setName("r{$callsign}{$raw_commit}") + ->setHref("r{$callsign}{$raw_commit}"); + $crumb_list[] = $crumb; return $crumb_list; } + $crumb = new PhabricatorCrumbView(); $view = $spec['view']; $path = null; @@ -247,7 +249,9 @@ abstract class DiffusionController extends PhabricatorController { break; case 'change': $view_name = 'Change'; - $crumb_list[] = phutil_escape_html($path).' ('.$commit_link.')'; + $crumb_list[] = $crumb->setRawName( + phutil_escape_html($path).' ('.$commit_link.')' + ); return $crumb_list; } @@ -255,19 +259,18 @@ abstract class DiffusionController extends PhabricatorController { 'action' => $view, ); + $crumb = id(new PhabricatorCrumbView()) + ->setName($view_name); if (!strlen($path)) { - $crumb_list[] = $view_name; + $crumb_list[] = $crumb; } else { - $crumb_list[] = phutil_render_tag( - 'a', + $crumb->setHref($drequest->generateURI( array( - 'href' => $drequest->generateURI( - array( - 'path' => '', - ) + $uri_params), - ), - $view_name); + 'path' => '', + ) + $uri_params) + ); + $crumb_list[] = $crumb; $path_parts = explode('/', $path); do { @@ -292,7 +295,8 @@ abstract class DiffusionController extends PhabricatorController { $path_sections[] = phutil_escape_html($last); $path_sections = '/'.implode('/', $path_sections); - $crumb_list[] = $path_sections; + $crumb_list[] = id(new PhabricatorCrumbView()) + ->setRawName($path_sections); } $last_crumb = array_pop($crumb_list); @@ -307,9 +311,13 @@ abstract class DiffusionController extends PhabricatorController { ) + $uri_params), ), 'Jump to HEAD'); - $last_crumb .= " @ {$commit_link} ({$jump_link})"; + $last_crumb->setRawName( + $last_crumb->getNameForRender() . " @ {$commit_link} ({$jump_link})" + ); } else if ($spec['view'] != 'lint') { - $last_crumb .= " @ HEAD"; + $last_crumb->setRawName( + $last_crumb->getNameForRender() . " @ HEAD" + ); } $crumb_list[] = $last_crumb; diff --git a/src/applications/diffusion/controller/DiffusionHistoryController.php b/src/applications/diffusion/controller/DiffusionHistoryController.php index ced077881a..3890d99d0c 100644 --- a/src/applications/diffusion/controller/DiffusionHistoryController.php +++ b/src/applications/diffusion/controller/DiffusionHistoryController.php @@ -39,13 +39,6 @@ final class DiffusionHistoryController extends DiffusionController { $content = array(); - $content[] = $this->buildCrumbs( - array( - 'branch' => true, - 'path' => true, - 'view' => 'history', - )); - if ($request->getBool('copies')) { $button_title = 'Hide Copies/Branches'; $copies_new = null; @@ -89,8 +82,15 @@ final class DiffusionHistoryController extends DiffusionController { $nav = $this->buildSideNav('history', false); $nav->appendChild($content); + $crumbs = $this->buildCrumbs( + array( + 'branch' => true, + 'path' => true, + 'view' => 'history', + )); + $nav->setCrumbs($crumbs); - return $this->buildStandardPageResponse( + return $this->buildApplicationPage( $nav, array( 'title' => 'history', diff --git a/src/applications/diffusion/controller/DiffusionLintController.php b/src/applications/diffusion/controller/DiffusionLintController.php index 467e1d6283..103fadb646 100644 --- a/src/applications/diffusion/controller/DiffusionLintController.php +++ b/src/applications/diffusion/controller/DiffusionLintController.php @@ -10,6 +10,7 @@ final class DiffusionLintController extends DiffusionController { if ($request->getStr('lint') !== null) { $controller = new DiffusionLintDetailsController($request); $controller->setDiffusionRequest($drequest); + $controller->setCurrentApplication($this->getCurrentApplication()); return $this->delegateToController($controller); } @@ -93,14 +94,6 @@ final class DiffusionLintController extends DiffusionController { $content = array(); - $content[] = $this->buildCrumbs( - array( - 'branch' => true, - 'path' => true, - 'view' => 'lint', - )); - - $link = null; if ($this->diffusionRequest) { $link = hsprintf( @@ -134,12 +127,22 @@ final class DiffusionLintController extends DiffusionController { ->appendChild($table); $title = array('Lint'); + $crumbs = $this->buildCrumbs( + array( + 'branch' => true, + 'path' => true, + 'view' => 'lint', + )); if ($this->diffusionRequest) { $title[] = $drequest->getCallsign(); - $content = $this->buildSideNav('lint', false)->appendChild($content); + $content = $this->buildSideNav('lint', false) + ->setCrumbs($crumbs) + ->appendChild($content); + } else { + array_unshift($content, $crumbs); } - return $this->buildStandardPageResponse( + return $this->buildApplicationPage( $content, array('title' => $title)); } diff --git a/src/applications/diffusion/controller/DiffusionLintDetailsController.php b/src/applications/diffusion/controller/DiffusionLintDetailsController.php index 3559584598..5eec9c507c 100644 --- a/src/applications/diffusion/controller/DiffusionLintDetailsController.php +++ b/src/applications/diffusion/controller/DiffusionLintDetailsController.php @@ -54,13 +54,6 @@ final class DiffusionLintDetailsController extends DiffusionController { $content = array(); - $content[] = $this->buildCrumbs( - array( - 'branch' => true, - 'path' => true, - 'view' => 'lint', - )); - $pager = id(new AphrontPagerView()) ->setPageSize($limit) ->setOffset($offset) @@ -86,8 +79,15 @@ final class DiffusionLintDetailsController extends DiffusionController { $nav = $this->buildSideNav('lint', false); $nav->appendChild($content); + $crumbs = $this->buildCrumbs( + array( + 'branch' => true, + 'path' => true, + 'view' => 'lint', + )); + $nav->setCrumbs($crumbs); - return $this->buildStandardPageResponse( + return $this->buildApplicationPage( $nav, array('title' => array( 'Lint', @@ -107,7 +107,12 @@ final class DiffusionLintDetailsController extends DiffusionController { $conn = $branch->establishConnection('r'); - $where = array(); + $where = array( + qsprintf( + $conn, + 'branchID = %d', + $branch->getID()) + ); if ($drequest->getPath() != '') { $is_dir = (substr($drequest->getPath(), -1) == '/'); $where[] = qsprintf( @@ -127,12 +132,9 @@ final class DiffusionLintDetailsController extends DiffusionController { $conn, 'SELECT * FROM %T - WHERE branchID = %d - AND %Q - ORDER BY path, code, line - LIMIT %d OFFSET %d', + WHERE %Q + ORDER BY path, code, line LIMIT %d OFFSET %d', PhabricatorRepository::TABLE_LINTMESSAGE, - $branch->getID(), implode(' AND ', $where), $limit, $offset); diff --git a/src/applications/diffusion/view/DiffusionView.php b/src/applications/diffusion/view/DiffusionView.php index 1ae1befba7..afa52d9d36 100644 --- a/src/applications/diffusion/view/DiffusionView.php +++ b/src/applications/diffusion/view/DiffusionView.php @@ -95,7 +95,9 @@ abstract class DiffusionView extends AphrontView { $text); } - final public static function linkCommit($repository, $commit) { + final public static function nameCommit( + PhabricatorRepository $repository, + $commit) { switch ($repository->getVersionControlSystem()) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: @@ -108,7 +110,15 @@ abstract class DiffusionView extends AphrontView { } $callsign = $repository->getCallsign(); - $commit_name = "r{$callsign}{$commit_name}"; + return "r{$callsign}{$commit_name}"; + } + + final public static function linkCommit( + PhabricatorRepository $repository, + $commit) { + + $commit_name = self::nameCommit($repository, $commit); + $callsign = $repository->getCallsign(); return phutil_render_tag( 'a', diff --git a/src/view/layout/PhabricatorCrumbView.php b/src/view/layout/PhabricatorCrumbView.php index 84d2230754..8d756281b7 100644 --- a/src/view/layout/PhabricatorCrumbView.php +++ b/src/view/layout/PhabricatorCrumbView.php @@ -6,12 +6,27 @@ final class PhabricatorCrumbView extends AphrontView { private $href; private $icon; private $isLastCrumb; + private $rawName; + + /** + * Allows for custom HTML inside the name field. + * + * NOTE: you must handle escaping user text if you use this method. + */ + public function setRawName($raw_name) { + $this->rawName = $raw_name; + return $this; + } public function setName($name) { $this->name = $name; return $this; } + public function getNameForRender() { + return nonempty($this->rawName, phutil_escape_html($this->name)); + } + public function setHref($href) { $this->href = $href; return $this; @@ -53,7 +68,7 @@ final class PhabricatorCrumbView extends AphrontView { array( 'class' => 'phabricator-crumb-name', ), - phutil_escape_html($this->name)); + $this->getNameForRender()); $divider = null; if (!$this->isLastCrumb) { diff --git a/webroot/rsrc/css/aphront/phabricator-nav-view.css b/webroot/rsrc/css/aphront/phabricator-nav-view.css index 29ac93ed51..b964bdf770 100644 --- a/webroot/rsrc/css/aphront/phabricator-nav-view.css +++ b/webroot/rsrc/css/aphront/phabricator-nav-view.css @@ -78,7 +78,7 @@ } .device-desktop .local-nav-collapsed .phabricator-nav-content { - margin-left: 2.5em !important; + margin-left: 0px !important; } .device .phabricator-nav-col {