Use more accurate matching in fix_gn_headers.py

build/fix_gn_headers.py

Index: build/fix_gn_headers.py
diff --git a/build/fix_gn_headers.py b/build/fix_gn_headers.py
index 5520a47815d1b5d969e8a3e2ea774b4292422e5a..231df267c7d7e92543b9d98786a5796dcad4ec35 100755
--- a/build/fix_gn_headers.py
+++ b/build/fix_gn_headers.py
@@ -17,6 +17,32 @@ import subprocess
 import sys
 
 
+def GitGrep(pattern):

[wychen, 2017/03/31 16:42:39]

Locally I use joblib.Memory to decorate this, so I extracted it.
Invalidation of the memorization is manual though.

+  p = subprocess.Popen(
+      ['git', 'grep', '-En', pattern, '--', '*.gn', '*.gni'],
+      stdout=subprocess.PIPE)
+  out, _ = p.communicate()
+  return out, p.returncode
+
+
+def ValidMatches(basename, cc, grep_lines):

[Nico, 2017/03/31 16:50:57]

function-level comment summarizing what this does

[wychen, 2017/04/01 05:32:04]

Done.

+  matches = []
+  for line in grep_lines:
+    gnfile, linenr, contents = line.split(':')
+    linenr = int(linenr)
+    new = re.sub(cc, basename, contents)
+    lines = open(gnfile).read().splitlines()
+    assert contents in lines[linenr - 1]
+    # Skip if it's already there. It could be before or after the match.
+    if lines[linenr] == new:
+      continue
+    if lines[linenr - 2] == new:
+      continue
+    print '    ', gnfile, linenr, new
+    matches.append((gnfile, linenr, new))
+  return matches
+
+
 def AddHeadersNextToCC(headers, skip_ambiguous=True):
   """Add header files next to the corresponding .cc files in GN files.
 
@@ -34,29 +60,31 @@ def AddHeadersNextToCC(headers, skip_ambiguous=True):
     basename = os.path.basename(filename)
     print filename
     cc = r'\b' + os.path.splitext(basename)[0] + r'\.(cc|cpp|mm)\b'
-    p = subprocess.Popen(
-        ['git', 'grep', '-En', cc + '"', '--', '*.gn', '*.gni'],
-        stdout=subprocess.PIPE)
-    out, _ = p.communicate()
-    if p.returncode != 0 or not out:
+    out, returncode = GitGrep('(/|")' + cc + '"')

[Nico, 2017/03/31 16:50:57]

nit: r'\b' instead of '(/|")'

[wychen, 2017/03/31 16:56:56]

'\b' is already there, but it's not enough. For example, "util.h" would match
"some-util.h".
Adding '/|"' makes it more accurate.

[wychen, 2017/03/31 16:58:08]

Ah. using sep might be better since we might want to run this on Windows
machine.

+    if returncode != 0 or not out:
       unhandled.append(filename)
       continue
 
-    if skip_ambiguous and len(out.splitlines()) > 1:
-      print '\n[WARNING] Ambiguous matching for', filename
-      print out
-      continue
+    matches = ValidMatches(basename, cc, out.splitlines())
 
-    for gnline in out.splitlines():
-      gnfile, linenr, contents = gnline.split(':')
-      linenr = int(linenr)
-      new = re.sub(cc, basename, contents)
-      lines = open(gnfile).read().splitlines()
-      # Skip if it's already there. It could be before or after the match.
-      if lines[linenr] == new:
+    if len(matches) == 0:
+      continue
+    if len(matches) > 1:
+      print '\n[WARNING] Ambiguous matching for', filename
+      for i in enumerate(matches, 1):
+        print '%d: %s' % (i[0], i[1])
+      print
+      if skip_ambiguous:
         continue
-      if lines[linenr - 2] == new:
+
+      picked = raw_input('Pick the matches ("2,3" for multiple): ')
+      try:
+        matches = [matches[int(i) - 1] for i in picked.split(',')]
+      except (ValueError, IndexError):
         continue
+
+    for match in matches:
+      gnfile, linenr, new = match
       print '  ', gnfile, linenr, new
       edits.setdefault(gnfile, {})[linenr] = new