recaptime.dev / phorge
@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.

Don't enable the "ScopeEngine" or try to identify scope context for diffs without context

Summary:
Depends on D20197. Ref T13161. We currently try to build a "ScopeEngine" even for diffs with no context (e.g., `git diff` instead of `git diff -U9999`).

Since we don't have any context, we won't really be able to figure out anything useful about scopes. Also, since ScopeEngine is pretty strict about what it accepts, we crash.

In these cases, just don't build a ScopeEngine.

Test Plan: Viewed a diff I copy/pasted with `git diff` instead of an `arc diff` / `git diff -U99999`, got a sensible diff with no context instead of a fatal.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13161

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

epriestley 1b832564 f1a035d5

+17 -7
+16 -6
src/applications/differential/render/DifferentialChangesetRenderer.php
··· 33 33 private $canMarkDone; 34 34 private $objectOwnerPHID; 35 35 private $highlightingDisabled; 36 - private $scopeEngine; 36 + private $scopeEngine = false; 37 37 private $depthOnlyLines; 38 38 39 39 private $oldFile = false; ··· 677 677 return $views; 678 678 } 679 679 680 - 681 680 final protected function getScopeEngine() { 682 - if (!$this->scopeEngine) { 683 - $line_map = $this->getNewLineTextMap(); 681 + if ($this->scopeEngine === false) { 682 + $hunk_starts = $this->getHunkStartLines(); 684 683 685 - $scope_engine = id(new PhabricatorDiffScopeEngine()) 686 - ->setLineTextMap($line_map); 684 + // If this change is missing context, don't try to identify scopes, since 685 + // we won't really be able to get anywhere. 686 + $has_multiple_hunks = (count($hunk_starts) > 1); 687 + $has_offset_hunks = (head_key($hunk_starts) != 1); 688 + $missing_context = ($has_multiple_hunks || $has_offset_hunks); 689 + 690 + if ($missing_context) { 691 + $scope_engine = null; 692 + } else { 693 + $line_map = $this->getNewLineTextMap(); 694 + $scope_engine = id(new PhabricatorDiffScopeEngine()) 695 + ->setLineTextMap($line_map); 696 + } 687 697 688 698 $this->scopeEngine = $scope_engine; 689 699 }
+1 -1
src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
··· 94 94 95 95 $context_text = null; 96 96 $context_line = null; 97 - if (!$is_last_block) { 97 + if (!$is_last_block && $scope_engine) { 98 98 $target_line = $new_lines[$ii + $len]['line']; 99 99 $context_line = $scope_engine->getScopeStart($target_line); 100 100 if ($context_line !== null) {