[git-cl] refactor hash tags support

Addressing comments in
https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/764974
that were sent after the CL landed.
Bug:
Change-Id: I6e0a583999e1c22f86d0f6b905aa5fae62b194d5
Reviewed-on: https://chromium-review.googlesource.com/775453Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

git_cl.py

@@ -3073,9 +3073,9 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
       refspec_opts.append('topic=%s' % options.topic)
 
     # Gerrit sorts hashtags, so order is not important.
-    hashtags = {self.SanitizeTag(t) for t in options.hashtags}
+    hashtags = {change_desc.sanitize_hash_tag(t) for t in options.hashtags}
     if not self.GetIssue():
-      hashtags.update(self.GetHashTags(change_desc.description))
+      hashtags.update(change_desc.get_hash_tags())
     refspec_opts += ['hashtag=%s' % t for t in sorted(hashtags)]
 
     refspec_suffix = ''
@@ -3142,40 +3142,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
 
     return 0
 
-  _STRIP_PREFIX_RGX = re.compile(
-      r'^(\s*(revert|reland)( "|:)?\s*)*', re.IGNORECASE)
-  _BRACKET_TAG_RGX = re.compile(r'\s*\[([^\[\]]+)\]')
-  _COLON_SEPARATED_TAG_RGX = re.compile(r'^([a-zA-Z0-9_\- ]+):')
-  _BAD_TAG_CHUNK_RGX = re.compile(r'[^a-zA-Z0-9]+')
-
-  @classmethod
-  def SanitizeTag(cls, tag):
-    """Returns a sanitized Gerrit hash tag. Can be used as parameter value."""
-    return cls._BAD_TAG_CHUNK_RGX.sub('-', tag).strip('-').lower()
-
-  @classmethod
-  def GetHashTags(cls, description):
-    """Extracts ans sanitizes a list of tags from a CL description."""
-
-    subject = description.split('\n', 1)[0]
-    subject = cls._STRIP_PREFIX_RGX.sub('', subject)
-
-    tags = []
-    start = 0
-    while True:
-      m = cls._BRACKET_TAG_RGX.match(subject, start)
-      if not m:
-        break
-      tags.append(cls.SanitizeTag(m.group(1)))
-      start = m.end()
-
-    if not tags:
-      # Try "Tag: " prefix.
-      m = cls._COLON_SEPARATED_TAG_RGX.match(subject)
-      if m:
-        tags.append(cls.SanitizeTag(m.group(1)))
-    return tags
-
   def _ComputeParent(self, remote, upstream_branch, custom_cl_base, force,
                      change_desc):
     """Computes parent of the generated commit to be uploaded to Gerrit.
@@ -3383,6 +3349,10 @@ class ChangeDescription(object):
   CC_LINE = r'^[ \t]*(CC)[ \t]*=[ \t]*(.*?)[ \t]*$'
   BUG_LINE = r'^[ \t]*(?:(BUG)[ \t]*=|Bug:)[ \t]*(.*?)[ \t]*$'
   CHERRY_PICK_LINE = r'^\(cherry picked from commit [a-fA-F0-9]{40}\)$'
+  STRIP_HASH_TAG_PREFIX = r'^(\s*(revert|reland)( "|:)?\s*)*'
+  BRACKET_HASH_TAG = r'\s*\[([^\[\]]+)\]'
+  COLON_SEPARATED_HASH_TAG = r'^([a-zA-Z0-9_\- ]+):'
+  BAD_HASH_TAG_CHUNK = r'[^a-zA-Z0-9]+'
 
   def __init__(self, description):
     self._description_lines = (description or '').strip().splitlines()
@@ -3555,6 +3525,37 @@ class ChangeDescription(object):
     cced = [match.group(2).strip() for match in matches if match]
     return cleanup_list(cced)
 
+  def get_hash_tags(self):
+    """Extracts and sanitizes a list of Gerrit hashtags."""
+    subject = (self._description_lines or ('',))[0]
+    subject = re.sub(
+        self.STRIP_HASH_TAG_PREFIX, '', subject, flags=re.IGNORECASE)
+
+    tags = []
+    start = 0
+    bracket_exp = re.compile(self.BRACKET_HASH_TAG)
+    while True:
+      m = bracket_exp.match(subject, start)
+      if not m:
+        break
+      tags.append(self.sanitize_hash_tag(m.group(1)))
+      start = m.end()
+
+    if not tags:
+      # Try "Tag: " prefix.
+      m = re.match(self.COLON_SEPARATED_HASH_TAG, subject)
+      if m:
+        tags.append(self.sanitize_hash_tag(m.group(1)))
+    return tags
+
+  @classmethod
+  def sanitize_hash_tag(cls, tag):
+    """Returns a sanitized Gerrit hash tag.
+
+    A sanitized hashtag can be used as a git push refspec parameter value.
+    """
+    return re.sub(cls.BAD_HASH_TAG_CHUNK, '-', tag).strip('-').lower()
+
   def update_with_git_number_footers(self, parent_hash, parent_msg, dest_ref):
     """Updates this commit description given the parent.
 

tests/git_cl_test.py

@@ -732,13 +732,6 @@ class TestGitCl(TestCase):
     ]
     self.assertTrue(git_cl.ask_for_explicit_yes('prompt'))
 
-  def test_ask_for_explicit_yes_true(self):
-    self.calls = [
-        (('ask_for_data', 'prompt [Yes/No]: '), 'yesish'),
-        (('ask_for_data', 'Please, type yes or no: '), 'nO'),
-    ]
-    self.assertFalse(git_cl.ask_for_explicit_yes('prompt'))
-
   def test_LoadCodereviewSettingsFromFile_gerrit(self):
     codereview_file = StringIO.StringIO('GERRIT_HOST: true')
     self.calls = [
@@ -1917,47 +1910,6 @@ class TestGitCl(TestCase):
     self.assertEquals(5, record_calls.times_called)
     self.assertEquals(0, ret)
 
-  def test_get_hash_tags(self):
-    cases = [
-      ('', []),
-      ('a', []),
-      ('[a]', ['a']),
-      ('[aa]', ['aa']),
-      ('[a ]', ['a']),
-      ('[a- ]', ['a']),
-      ('[a- b]', ['a-b']),
-      ('[a--b]', ['a-b']),
-      ('[a', []),
-      ('[a]x', ['a']),
-      ('[aa]x', ['aa']),
-      ('[a b]', ['a-b']),
-      ('[a  b]', ['a-b']),
-      ('[a__b]', ['a-b']),
-      ('[a] x', ['a']),
-      ('[a][b]', ['a', 'b']),
-      ('[a] [b]', ['a', 'b']),
-      ('[a][b]x', ['a', 'b']),
-      ('[a][b] x', ['a', 'b']),
-      ('[a]\n[b]', ['a']),
-      ('[a\nb]', []),
-      ('[a][', ['a']),
-      ('Revert "[a] feature"', ['a']),
-      ('Reland "[a] feature"', ['a']),
-      ('Revert: [a] feature', ['a']),
-      ('Reland: [a] feature', ['a']),
-      ('Revert "Reland: [a] feature"', ['a']),
-      ('Foo: feature', ['foo']),
-      ('Foo Bar: feature', ['foo-bar']),
-      ('Revert "Foo bar: feature"', ['foo-bar']),
-      ('Reland "Foo bar: feature"', ['foo-bar']),
-    ]
-    for desc, expected in cases:
-      actual = git_cl._GerritChangelistImpl.GetHashTags(desc)
-      self.assertEqual(
-          actual,
-          expected,
-          'GetHashTags(%r) == %r, expected %r' % (desc, actual, expected))
-
   def test_gerrit_change_id(self):
     self.calls = [
         ((['git', 'write-tree'], ),
@@ -2039,6 +1991,48 @@ class TestGitCl(TestCase):
       actual.append(obj.description)
     self.assertEqual(expected, actual)
 
+  def test_get_hash_tags(self):
+    cases = [
+      ('', []),
+      ('a', []),
+      ('[a]', ['a']),
+      ('[aa]', ['aa']),
+      ('[a ]', ['a']),
+      ('[a- ]', ['a']),
+      ('[a- b]', ['a-b']),
+      ('[a--b]', ['a-b']),
+      ('[a', []),
+      ('[a]x', ['a']),
+      ('[aa]x', ['aa']),
+      ('[a b]', ['a-b']),
+      ('[a  b]', ['a-b']),
+      ('[a__b]', ['a-b']),
+      ('[a] x', ['a']),
+      ('[a][b]', ['a', 'b']),
+      ('[a] [b]', ['a', 'b']),
+      ('[a][b]x', ['a', 'b']),
+      ('[a][b] x', ['a', 'b']),
+      ('[a]\n[b]', ['a']),
+      ('[a\nb]', []),
+      ('[a][', ['a']),
+      ('Revert "[a] feature"', ['a']),
+      ('Reland "[a] feature"', ['a']),
+      ('Revert: [a] feature', ['a']),
+      ('Reland: [a] feature', ['a']),
+      ('Revert "Reland: [a] feature"', ['a']),
+      ('Foo: feature', ['foo']),
+      ('Foo Bar: feature', ['foo-bar']),
+      ('Revert "Foo bar: feature"', ['foo-bar']),
+      ('Reland "Foo bar: feature"', ['foo-bar']),
+    ]
+    for desc, expected in cases:
+      change_desc = git_cl.ChangeDescription(desc)
+      actual = change_desc.get_hash_tags()
+      self.assertEqual(
+          actual,
+          expected,
+          'GetHashTags(%r) == %r, expected %r' % (desc, actual, expected))
+
   def test_get_target_ref(self):
     # Check remote or remote branch not present.
     self.assertEqual(None, git_cl.GetTargetRef('origin', None, 'master'))