From 6d13b9fb7a263e56e75a7063894e1a6bc7d79d8b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 25 Apr 2019 15:26:48 -0700
Subject: [PATCH] Make very minor generality improvements to the scope selector

Summary:
See PHI985. I think we pretty much need to start applying language-specific rules, but we can apply at least one more relatively language-agnostic rule: don't match lines which are indented 3+ levels.

In C++, we may have symbols like this:

```
class X {
  public:
    int m() { ... }
}
```

..but I believe no mainstream language puts symbol definitions 3+ levels deep.

Also clean up some of the tab handling very slightly.

Test Plan: Tests pass, looked at some C++ code and got slightly better (but still not great) matches.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20479
---
 .../diff/PhabricatorDiffScopeEngine.php       | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/infrastructure/diff/PhabricatorDiffScopeEngine.php b/src/infrastructure/diff/PhabricatorDiffScopeEngine.php
index 5ea1ec5021..ede095f266 100644
--- a/src/infrastructure/diff/PhabricatorDiffScopeEngine.php
+++ b/src/infrastructure/diff/PhabricatorDiffScopeEngine.php
@@ -83,6 +83,12 @@ final class PhabricatorDiffScopeEngine
         continue;
       }
 
+      // Don't match context lines which are too deeply indented, since they
+      // are very unlikely to be symbol definitions.
+      if ($depth > 2) {
+        continue;
+      }
+
       // The depth is the same as (or greater than) the depth we started with,
       // so this isn't a possible match.
       if ($depth >= $line_depth) {
@@ -109,11 +115,14 @@ final class PhabricatorDiffScopeEngine
     return $this->lineDepthMap;
   }
 
+  private function getTabWidth() {
+    // TODO: This should be configurable once we handle tab widths better.
+    return 2;
+  }
+
   private function newLineDepthMap() {
     $text_map = $this->getLineTextMap();
-
-    // TODO: This should be configurable once we handle tab widths better.
-    $tab_width = 2;
+    $tab_width = $this->getTabWidth();
 
     $depth_map = array();
     foreach ($text_map as $line_number => $line_text) {
@@ -143,9 +152,8 @@ final class PhabricatorDiffScopeEngine
       }
 
       // Round down to cheat our way through the " *" parts of docblock
-      // comments. This is generally a reasonble heuristic because odd tab
-      // widths are exceptionally rare.
-      $depth = ($count >> 1);
+      // comments.
+      $depth = (int)floor($count / $tab_width);
 
       $depth_map[$line_number] = $depth;
     }