Fix R= line rewriter to handle TBRs well.

git_cl.py

 #!/usr/bin/env python
 # 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.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld."""
 
 from distutils.version import LooseVersion
@@ skipping 840 matching lines @@
   R_LINE = r'^[ \t]*(TBR|R)[ \t]*=[ \t]*(.*?)[ \t]*$'
 
   def __init__(self, description):
     self._description = (description or '').strip()
 
   @property
   def description(self):
     return self._description
 
   def update_reviewers(self, reviewers):
-    """Rewrites the R=/TBR= line(s) as a single line."""
+    """Rewrites the R=/TBR= line(s) as a single line each."""
     assert isinstance(reviewers, list), reviewers
     if not reviewers:
       return
     regexp = re.compile(self.R_LINE, re.MULTILINE)
     matches = list(regexp.finditer(self._description))
-    is_tbr = any(m.group(1) == 'TBR' for m in matches)
+    r_names = []
+    tbr_names = []
+    for line in matches:
+      is_tbr = line.group(1) == 'TBR'
+      people = cleanup_list([line.group(2).strip()])
+      if is_tbr:
+        tbr_names += people
+      else:
+        r_names += people
     if len(matches) > 1:
       # Erase all except the first one.
       for i in xrange(len(matches) - 1, 0, -1):
         self._description = (
             self._description[:matches[i].start()] +
             self._description[matches[i].end():])
+    # Copy the names from the description into the official list.
+    for name in r_names:
+      if name not in reviewers:
+        reviewers.append(name)
 
-    if is_tbr:
-      new_r_line = 'TBR=' + ', '.join(reviewers)
-    else:
-      new_r_line = 'R=' + ', '.join(reviewers)
+    new_r_line = 'R=' + ', '.join(reviewers)

[M-A Ruel, 2013/08/23 15:22:53]

I'd also skip it unless "if reviewers:"

[agable, 2013/08/23 18:43:00]

I ended up refactoring most of the ChangeDescription class to make more sense.
This change is included in that.

+    if tbr_names:
+      new_r_line += '\nTBR=' + ', '.join(tbr_names)
 
     if matches:
       self._description = (
         self._description[:matches[0].start()] + new_r_line +
         self._description[matches[0].end():]).strip()
     else:
       self.append_footer(new_r_line)
 
   def prompt(self):
     """Asks the user to update the description."""
@@ skipping 1308 matching lines @@
         ('AppEngine is misbehaving and returned HTTP %d, again. Keep faith '
           'and retry or visit go/isgaeup.\n%s') % (e.code, str(e)))
 
 
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   colorama.init()
   sys.exit(main(sys.argv[1:]))