From 99ac1c580824cf3a763a015ed43397743a579530 Mon Sep 17 00:00:00 2001
From: "maruel@chromium.org"
 <maruel@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Mon, 16 Jan 2012 14:52:12 +0000
Subject: [PATCH] Dedupe codereview.settings parsing code. Add more testing.

So any modification to the defaults is coded at only one place.

TBR=nsylvain@chromium.org
BUG=107838
TEST=


Review URL: http://codereview.chromium.org/9214002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@117853 0039d316-1c4b-4281-b951-d872f2087c98
---
 gcl.py                      | 11 ++---------
 gclient_utils.py            | 14 +++++++++++++-
 git_cl.py                   |  7 +------
 tests/gclient_utils_test.py | 20 ++++++++++++++++++--
 4 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/gcl.py b/gcl.py
index a5a56d17a8..3a17fc8201 100755
--- a/gcl.py
+++ b/gcl.py
@@ -208,15 +208,8 @@ def GetCodeReviewSetting(key):
   if '__just_initialized' not in CODEREVIEW_SETTINGS:
     settings_file = GetCachedFile(CODEREVIEW_SETTINGS_FILE)
     if settings_file:
-      for line in settings_file.splitlines():
-        if not line or line.startswith('#'):
-          continue
-        if not ':' in line:
-          raise gclient_utils.Error(
-              '%s is invalid, please fix. It\'s content:\n\n%s' %
-                  (CODEREVIEW_SETTINGS_FILE, settings_file))
-        k, v = line.split(':', 1)
-        CODEREVIEW_SETTINGS[k.strip()] = v.strip()
+      CODEREVIEW_SETTINGS.update(
+          gclient_utils.ParseCodereviewSettingsContent(settings_file))
     CODEREVIEW_SETTINGS.setdefault('__just_initialized', None)
   return CODEREVIEW_SETTINGS.get(key, "")
 
diff --git a/gclient_utils.py b/gclient_utils.py
index bad86915b3..e5784ac473 100644
--- a/gclient_utils.py
+++ b/gclient_utils.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -732,3 +732,15 @@ def RunEditor(content, git):
     return FileRead(filename)
   finally:
     os.remove(filename)
+
+
+def ParseCodereviewSettingsContent(content):
+  """Process a codereview.settings file properly."""
+  lines = (l for l in content.splitlines() if not l.strip().startswith("#"))
+  try:
+    keyvals = dict([x.strip() for x in l.split(':', 1)] for l in lines if l)
+  except ValueError:
+    raise Error(
+        'Failed to process settings, please fix. Content:\n\n%s' % content)
+  # TODO(maruel): Post-process
+  return keyvals
diff --git a/git_cl.py b/git_cl.py
index eb93403f7a..6af8bcbe53 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -717,12 +717,7 @@ def FindCodereviewSettingsFile(filename='codereview.settings'):
 
 def LoadCodereviewSettingsFromFile(fileobj):
   """Parse a codereview.settings file and updates hooks."""
-  keyvals = {}
-  for line in fileobj.read().splitlines():
-    if not line or line.startswith("#"):
-      continue
-    k, v = line.split(": ", 1)
-    keyvals[k] = v
+  keyvals = gclient_utils.ParseCodereviewSettingsContent(fileobj.read())
 
   def SetProperty(name, setting, unset_error_ok=False):
     fullname = 'rietveld.' + name
diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py
index 023f7e9798..937e43a7cb 100755
--- a/tests/gclient_utils_test.py
+++ b/tests/gclient_utils_test.py
@@ -1,5 +1,5 @@
 #!/usr/bin/env python
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -33,7 +33,8 @@ class GclientUtilsUnittest(GclientUtilBase):
         'FileWrite', 'FindFileUpwards', 'FindGclientRoot',
         'GetGClientRootAndEntries', 'GetEditor', 'IsDateRevision',
         'MakeDateRevision', 'MakeFileAutoFlush', 'MakeFileAnnotated',
-        'PathDifference', 'PrintableObject', 'RemoveDirectory', 'RunEditor',
+        'PathDifference', 'ParseCodereviewSettingsContent',
+        'PrintableObject', 'RemoveDirectory', 'RunEditor',
         'SplitUrlRevision', 'SyntaxErrorToError', 'Wrapper', 'WorkItem',
         'errno', 'lockedmethod', 'logging', 'os', 'Queue', 're', 'rmtree',
         'safe_makedirs', 'stat', 'subprocess2', 'sys', 'tempfile', 'threading',
@@ -170,6 +171,21 @@ class GClientUtilsTest(trial_dir.TestCase):
     os.chmod(l2, 0)
     os.chmod(l1, 0)
 
+  def testParseCodereviewSettingsContent(self):
+    expected = {
+        'Foo': 'bar:baz',
+        'Second': 'value',
+    }
+    content = (
+        '# bleh\n'
+        '\t# foo : bar\n'
+        'Foo:bar:baz\n'
+        ' Second : value \n\r'
+        '#inconsistency'
+    )
+    self.assertEquals(
+        expected, gclient_utils.ParseCodereviewSettingsContent(content))
+
 
 if __name__ == '__main__':
   import unittest