[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

eric at webkit.org eric at webkit.org
Wed Dec 22 14:37:00 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 2830d6e8eb20a0a4ab03665c53a4e8069bc65109
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Thu Oct 14 06:55:51 2010 +0000

    2010-10-13  Eric Seidel  <eric at webkit.org>
    
            Reviewed by Adam Barth.
    
            beat diff_parser with the ugly stick
            https://bugs.webkit.org/show_bug.cgi?id=47626
    
            * Scripts/webkitpy/common/checkout/diff_parser.py:
            * Scripts/webkitpy/style/patchreader.py:
            * Scripts/webkitpy/style/patchreader_unittest.py:
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@69744 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index fabc2f8..b28db9e 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -2,6 +2,17 @@
 
         Reviewed by Adam Barth.
 
+        beat diff_parser with the ugly stick
+        https://bugs.webkit.org/show_bug.cgi?id=47626
+
+        * Scripts/webkitpy/common/checkout/diff_parser.py:
+        * Scripts/webkitpy/style/patchreader.py:
+        * Scripts/webkitpy/style/patchreader_unittest.py:
+
+2010-10-13  Eric Seidel  <eric at webkit.org>
+
+        Reviewed by Adam Barth.
+
         Break LayoutTestResults out into its own file
         https://bugs.webkit.org/show_bug.cgi?id=47637
 
diff --git a/WebKitTools/Scripts/webkitpy/common/checkout/diff_parser.py b/WebKitTools/Scripts/webkitpy/common/checkout/diff_parser.py
index d8ebae6..a6ea756 100644
--- a/WebKitTools/Scripts/webkitpy/common/checkout/diff_parser.py
+++ b/WebKitTools/Scripts/webkitpy/common/checkout/diff_parser.py
@@ -33,9 +33,13 @@ import re
 
 _log = logging.getLogger("webkitpy.common.checkout.diff_parser")
 
+
+# FIXME: This is broken. We should compile our regexps up-front
+# instead of using a custom cache.
 _regexp_compile_cache = {}
 
 
+# FIXME: This function should be removed.
 def match(pattern, string):
     """Matches the string with the pattern, caching the compiled regexp."""
     if not pattern in _regexp_compile_cache:
@@ -43,12 +47,15 @@ def match(pattern, string):
     return _regexp_compile_cache[pattern].match(string)
 
 
+# FIXME: This belongs on DiffParser (e.g. as to_svn_diff()).
 def git_diff_to_svn_diff(line):
     """Converts a git formatted diff line to a svn formatted line.
 
     Args:
       line: A string representing a line of the diff.
     """
+    # FIXME: This list should be a class member on DiffParser.
+    # These regexp patterns should be compiled once instead of every time.
     conversion_patterns = (("^diff --git \w/(.+) \w/(?P<FilePath>.+)", lambda matched: "Index: " + matched.group('FilePath') + "\n"),
                            ("^new file.*", lambda matched: "\n"),
                            ("^index [0-9a-f]{7}\.\.[0-9a-f]{7} [0-9]{6}", lambda matched: "===================================================================\n"),
@@ -62,6 +69,7 @@ def git_diff_to_svn_diff(line):
     return line
 
 
+# FIXME: This method belongs on DiffParser
 def get_diff_converter(first_diff_line):
     """Gets a converter function of diff lines.
 
@@ -80,7 +88,7 @@ _DECLARED_FILE_PATH = 2
 _PROCESSING_CHUNK = 3
 
 
-class DiffFile:
+class DiffFile(object):
     """Contains the information for one file in a patch.
 
     The field "lines" is a list which contains tuples in this format:
@@ -88,6 +96,13 @@ class DiffFile:
     If deleted_line_number is zero, it means this line is newly added.
     If new_line_number is zero, it means this line is deleted.
     """
+    # FIXME: Tuples generally grow into classes.  We should consider
+    # adding a DiffLine object.
+
+    def added_or_modified_line_numbers(self):
+        # This logic was moved from patchreader.py, but may not be
+        # the right API for this object long-term.
+        return [line[1] for line in self.lines if not line[0]]
 
     def __init__(self, filename):
         self.filename = filename
@@ -103,13 +118,14 @@ class DiffFile:
         self.lines.append((deleted_line_number, new_line_number, line))
 
 
-class DiffParser:
+class DiffParser(object):
     """A parser for a patch file.
 
     The field "files" is a dict whose key is the filename and value is
     a DiffFile object.
     """
 
+    # FIXME: This function is way too long and needs to be broken up.
     def __init__(self, diff_input):
         """Parses a diff.
 
diff --git a/WebKitTools/Scripts/webkitpy/style/patchreader.py b/WebKitTools/Scripts/webkitpy/style/patchreader.py
index 576504a..f44839d 100644
--- a/WebKitTools/Scripts/webkitpy/style/patchreader.py
+++ b/WebKitTools/Scripts/webkitpy/style/patchreader.py
@@ -37,7 +37,6 @@ _log = logging.getLogger("webkitpy.style.patchreader")
 
 
 class PatchReader(object):
-
     """Supports checking style in patches."""
 
     def __init__(self, text_file_reader):
@@ -53,28 +52,15 @@ class PatchReader(object):
         """Check style in the given patch."""
         patch_files = DiffParser(patch_string.splitlines()).files
 
-        # The diff variable is a DiffFile instance.
-        for path, diff in patch_files.iteritems():
-            line_numbers = set()
-            for line in diff.lines:
-                # When deleted line is not set, it means that
-                # the line is newly added (or modified).
-                if not line[0]:
-                    line_numbers.add(line[1])
-
-            _log.debug('Found %s new or modified lines in: %s'
-                       % (len(line_numbers), path))
+        for path, diff_file in patch_files.iteritems():
+            line_numbers = diff_file.added_or_modified_line_numbers()
+            _log.debug('Found %s new or modified lines in: %s' % (len(line_numbers), path))
 
-            # If line_numbers is empty, the file has no new or
-            # modified lines.  In this case, we don't check the file
-            # because we'll never output errors for the file.
-            # This optimization also prevents the program from exiting
-            # due to a deleted file.
-            if line_numbers:
-                self._text_file_reader.process_file(file_path=path,
-                                                    line_numbers=line_numbers)
-            else:
-                # We don't check the file which contains deleted lines only
-                # but would like to treat it as to be processed so that
-                # we count up number of such files.
+            if not line_numbers:
+                # Don't check files which contain only deleted lines
+                # as they can never add style errors. However, mark them as
+                # processed so that we count up number of such files.
                 self._text_file_reader.count_delete_only_file()
+                continue
+
+            self._text_file_reader.process_file(file_path=path, line_numbers=line_numbers)
diff --git a/WebKitTools/Scripts/webkitpy/style/patchreader_unittest.py b/WebKitTools/Scripts/webkitpy/style/patchreader_unittest.py
index 2453c6b..b121082 100644
--- a/WebKitTools/Scripts/webkitpy/style/patchreader_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/style/patchreader_unittest.py
@@ -78,7 +78,7 @@ index ef65bee..e3db70e 100644
  # Required for Python to search this directory for module files
 +# New line
 """)
-        self._assert_checked([("__init__.py", set([2]))], 0)
+        self._assert_checked([("__init__.py", [2])], 0)
 
     def test_check_patch_with_deletion(self):
         self._call_check_patch("""Index: __init__.py

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list