diff --git a/git_cl.py b/git_cl.py index 487d9c604c..523dc2c427 100755 --- a/git_cl.py +++ b/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. diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 8bd07ef1f7..c9e28f2834 100755 --- a/tests/git_cl_test.py +++ b/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'))