@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Use ChangesetListView on Differential standalone view

Summary:
Fixes T4452. Ref T2009. There's a hierarchy of changeset rendering power: only low-level calls, use of ChangesetDetailView, then use of ChangesetListView (a list of DetailViews).

Prior to work here, the various changeset rendering controllers got their hands dirty to varying degrees, with some using only the lowest-level rendering pipeline:

- Phriction: no view (lowest level)
- Diffusion: DetailView
- Differential Changeset: DetailView
- Differential Diff: ListView
- Differential Revision: ListView

I brought Phriction up to use DetailView, but want to bring everything all the way up to use ListView. Each composition layer adds more features to diff browsing. In particular, this change enables "Highlight As", switching 1up vs 2up, adding inlines, etc., on the standalone view.

Test Plan:
- Viewed a changeset standalone. Could change highlighting, switch 1up vs 2up, add and edit inlines, etc.
- Viewed a revision; no behavioral changes.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4452, T2009

Differential Revision: https://secure.phabricator.com/D12012

+103 -73
+31 -50
src/applications/differential/controller/DifferentialChangesetViewController.php
··· 147 147 list($range_s, $range_e, $mask) = 148 148 DifferentialChangesetParser::parseRangeSpecification($spec); 149 149 150 - $parser = new DifferentialChangesetParser(); 151 - $parser->setCoverage($coverage); 152 - $parser->setChangeset($changeset); 153 - $parser->setRenderingReference($rendering_reference); 154 - $parser->setRenderCacheKey($render_cache_key); 155 - $parser->setRightSideCommentMapping($right_source, $right_new); 156 - $parser->setLeftSideCommentMapping($left_source, $left_new); 150 + $parser = id(new DifferentialChangesetParser()) 151 + ->setCoverage($coverage) 152 + ->setChangeset($changeset) 153 + ->setRenderingReference($rendering_reference) 154 + ->setRenderCacheKey($render_cache_key) 155 + ->setRightSideCommentMapping($right_source, $right_new) 156 + ->setLeftSideCommentMapping($left_source, $left_new); 157 157 158 158 $parser->readParametersFromRequest($request); 159 159 ··· 200 200 } 201 201 202 202 $engine->process(); 203 - $parser->setMarkupEngine($engine); 204 - $parser->setUser($request->getUser()); 203 + 204 + $parser 205 + ->setUser($request->getUser()) 206 + ->setMarkupEngine($engine) 207 + ->setShowEditAndReplyLinks(true) 208 + ->setRange($range_s, $range_e) 209 + ->setMask($mask); 205 210 206 211 if ($request->isAjax()) { 207 - $parser->setShowEditAndReplyLinks(true); 208 - } else { 209 - $parser->setShowEditAndReplyLinks(false); 210 - } 212 + $mcov = $parser->renderModifiedCoverage(); 211 213 212 - $output = $parser->render($range_s, $range_e, $mask); 213 - 214 - $mcov = $parser->renderModifiedCoverage(); 215 - 216 - if ($request->isAjax()) { 217 214 $coverage = array( 218 215 'differential-mcoverage-'.md5($changeset->getFilename()) => $mcov, 219 216 ); 220 217 221 218 return id(new PhabricatorChangesetResponse()) 222 - ->setRenderedChangeset($output) 219 + ->setRenderedChangeset($parser->renderChangeset()) 223 220 ->setCoverage($coverage); 224 221 } 225 222 226 - // TODO: [HTML] Clean up DifferentialChangesetParser output, but it's 227 - // undergoing like six kinds of refactoring anyway. 228 - $output = phutil_safe_html($output); 223 + $diff = $changeset->getDiff(); 229 224 230 - $detail = id(new DifferentialChangesetDetailView()) 225 + $detail = id(new DifferentialChangesetListView()) 231 226 ->setUser($this->getViewer()) 232 - ->setChangeset($changeset) 233 - ->setRenderingRef($rendering_reference) 227 + ->setChangesets(array($changeset)) 228 + ->setVisibleChangesets(array($changeset)) 229 + ->setRenderingReferences(array($rendering_reference)) 234 230 ->setRenderURI('/differential/changeset/') 235 - ->setRenderer($parser->getRenderer()->getRendererKey()) 236 - ->appendChild($output) 237 - ->setVsChangesetID($left_source); 238 - 239 - Javelin::initBehavior('differential-populate', array( 240 - 'changesetViewIDs' => array($detail->getID()), 241 - )); 242 - 243 - Javelin::initBehavior('differential-comment-jump', array()); 231 + ->setDiff($diff) 232 + ->setTitle(pht('Standalone View')) 233 + ->setParser($parser); 244 234 245 - $panel = new DifferentialPrimaryPaneView(); 246 - $panel->appendChild( 247 - phutil_tag( 248 - 'div', 249 - array( 250 - 'class' => 'differential-review-stage', 251 - 'id' => 'differential-review-stage', 252 - ), 253 - $detail->render())); 235 + $revision_id = $diff->getRevisionID(); 236 + if ($revision_id) { 237 + $detail->setInlineCommentControllerURI( 238 + '/differential/comment/inline/edit/'.$revision_id.'/'); 239 + } 254 240 255 241 $crumbs = $this->buildApplicationCrumbs(); 256 242 257 - $revision_id = $changeset->getDiff()->getRevisionID(); 258 243 if ($revision_id) { 259 244 $crumbs->addTextCrumb('D'.$revision_id, '/D'.$revision_id); 260 245 } 261 246 262 - $diff_id = $changeset->getDiff()->getID(); 247 + $diff_id = $diff->getID(); 263 248 if ($diff_id) { 264 249 $crumbs->addTextCrumb( 265 250 pht('Diff %d', $diff_id), ··· 268 253 269 254 $crumbs->addTextCrumb($changeset->getDisplayFilename()); 270 255 271 - $box = id(new PHUIObjectBoxView()) 272 - ->setHeaderText(pht('Standalone View')) 273 - ->appendChild($panel); 274 - 275 256 return $this->buildApplicationPage( 276 257 array( 277 258 $crumbs, 278 - $box, 259 + $detail, 279 260 ), 280 261 array( 281 262 'title' => pht('Changeset View'),
+19
src/applications/differential/parser/DifferentialChangesetParser.php
··· 45 45 private $highlightAs; 46 46 private $showEditAndReplyLinks = true; 47 47 48 + private $rangeStart; 49 + private $rangeEnd; 50 + private $mask; 51 + 52 + public function setRange($start, $end) { 53 + $this->rangeStart = $start; 54 + $this->rangeEnd = $end; 55 + return $this; 56 + } 57 + 58 + public function setMask(array $mask) { 59 + $this->mask = $mask; 60 + return $this; 61 + } 62 + 63 + public function renderChangeset() { 64 + return $this->render($this->rangeStart, $this->rangeEnd, $this->mask); 65 + } 66 + 48 67 public function setShowEditAndReplyLinks($bool) { 49 68 $this->showEditAndReplyLinks = $bool; 50 69 return $this;
+11
src/applications/differential/view/DifferentialChangesetDetailView.php
··· 12 12 private $whitespace; 13 13 private $renderingRef; 14 14 private $autoload; 15 + private $loaded; 15 16 private $renderer; 16 17 17 18 public function setAutoload($autoload) { ··· 21 22 22 23 public function getAutoload() { 23 24 return $this->autoload; 25 + } 26 + 27 + public function setLoaded($loaded) { 28 + $this->loaded = $loaded; 29 + return $this; 30 + } 31 + 32 + public function getLoaded() { 33 + return $this->loaded; 24 34 } 25 35 26 36 public function setRenderingRef($rendering_ref) { ··· 213 223 'renderer' => $this->getRenderer(), 214 224 'ref' => $this->getRenderingRef(), 215 225 'autoload' => $this->getAutoload(), 226 + 'loaded' => $this->getLoaded(), 216 227 ), 217 228 'class' => $class, 218 229 'id' => $id,
+41 -23
src/applications/differential/view/DifferentialChangesetListView.php
··· 20 20 private $vsMap = array(); 21 21 22 22 private $title; 23 + private $parser; 24 + 25 + public function setParser(DifferentialChangesetParser $parser) { 26 + $this->parser = $parser; 27 + return $this; 28 + } 29 + 30 + public function getParser() { 31 + return $this->parser; 32 + } 23 33 24 34 public function setTitle($title) { 25 35 $this->title = $title; ··· 168 178 $detail->setVsChangesetID(idx($this->vsMap, $changeset->getID())); 169 179 $detail->setEditable(true); 170 180 $detail->setRenderingRef($ref); 171 - $detail->setAutoload(isset($this->visibleChangesets[$key])); 172 181 173 182 $detail->setRenderURI($this->renderURI); 174 183 $detail->setWhitespace($this->whitespace); 175 184 $detail->setRenderer($renderer); 176 185 177 - if (isset($this->visibleChangesets[$key])) { 178 - $load = 'Loading...'; 186 + if ($this->getParser()) { 187 + $detail->appendChild($this->getParser()->renderChangeset()); 188 + $detail->setLoaded(true); 179 189 } else { 180 - $load = javelin_tag( 181 - 'a', 182 - array( 183 - 'class' => 'button grey', 184 - 'href' => '#'.$uniq_id, 185 - 'sigil' => 'differential-load', 186 - 'meta' => array( 187 - 'id' => $detail->getID(), 188 - 'kill' => true, 190 + $detail->setAutoload(isset($this->visibleChangesets[$key])); 191 + if (isset($this->visibleChangesets[$key])) { 192 + $load = 'Loading...'; 193 + } else { 194 + $load = javelin_tag( 195 + 'a', 196 + array( 197 + 'class' => 'button grey', 198 + 'href' => '#'.$uniq_id, 199 + 'sigil' => 'differential-load', 200 + 'meta' => array( 201 + 'id' => $detail->getID(), 202 + 'kill' => true, 203 + ), 204 + 'mustcapture' => true, 189 205 ), 190 - 'mustcapture' => true, 191 - ), 192 - pht('Load File')); 206 + pht('Load File')); 207 + } 208 + $detail->appendChild( 209 + phutil_tag( 210 + 'div', 211 + array( 212 + 'id' => $uniq_id, 213 + ), 214 + phutil_tag( 215 + 'div', 216 + array('class' => 'differential-loading'), 217 + $load))); 193 218 } 194 - $detail->appendChild( 195 - phutil_tag( 196 - 'div', 197 - array( 198 - 'id' => $uniq_id, 199 - ), 200 - phutil_tag('div', array('class' => 'differential-loading'), $load))); 219 + 201 220 $output[] = $detail->render(); 202 - 203 221 $ids[] = $detail->getID(); 204 222 } 205 223
+1
webroot/rsrc/js/application/differential/ChangesetViewManager.js
··· 23 23 this._renderer = data.renderer; 24 24 this._highlight = data.highlight; 25 25 this._encoding = data.encoding; 26 + this._loaded = data.loaded; 26 27 }, 27 28 28 29 members: {