diff --git a/git_cl.py b/git_cl.py index 9c3059ade..a08141bbe 100755 --- a/git_cl.py +++ b/git_cl.py @@ -2692,13 +2692,41 @@ class ChangeDescription(object): self.set_description(clean_lines) def append_footer(self, line): - if self._description_lines: - # Add an empty line if either the last line or the new line isn't a tag. - last_line = self._description_lines[-1] - if (not presubmit_support.Change.TAG_LINE_RE.match(last_line) or - not presubmit_support.Change.TAG_LINE_RE.match(line)): - self._description_lines.append('') - self._description_lines.append(line) + """Adds a footer line to the description. + + Differentiates legacy "KEY=xxx" footers (used to be called tags) and + Gerrit's footers in the form of "Footer-Key: footer any value" and ensures + that Gerrit footers are always at the end. + """ + parsed_footer_line = git_footers.parse_footer(line) + if parsed_footer_line: + # Line is a gerrit footer in the form: Footer-Key: any value. + # Thus, must be appended observing Gerrit footer rules. + self.set_description( + git_footers.add_footer(self.description, + key=parsed_footer_line[0], + value=parsed_footer_line[1])) + return + + if not self._description_lines: + self._description_lines.append(line) + return + + top_lines, gerrit_footers, _ = git_footers.split_footers(self.description) + if gerrit_footers: + # git_footers.split_footers ensures that there is an empty line before + # actual (gerrit) footers, if any. We have to keep it that way. + assert top_lines and top_lines[-1] == '' + top_lines, separator = top_lines[:-1], top_lines[-1:] + else: + separator = [] # No need for separator if there are no gerrit_footers. + + prev_line = top_lines[-1] if top_lines else '' + if (not presubmit_support.Change.TAG_LINE_RE.match(prev_line) or + not presubmit_support.Change.TAG_LINE_RE.match(line)): + top_lines.append('') + top_lines.append(line) + self._description_lines = top_lines + separator + gerrit_footers def get_reviewers(self): """Retrieves the list of reviewers.""" diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 008400188..42ffba4f5 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -1022,6 +1022,25 @@ class TestGitCl(TestCase): change_id = git_cl.GenerateGerritChangeId('line1\nline2\n') self.assertEqual(change_id, 'Ihashchange') + def test_desecription_append_footer(self): + for init_desc, footer_line, expected_desc in [ + # Use unique desc first lines for easy test failure identification. + ('foo', 'R=one', 'foo\n\nR=one'), + ('foo\n\nR=one', 'BUG=', 'foo\n\nR=one\nBUG='), + ('foo\n\nR=one', 'Change-Id: Ixx', 'foo\n\nR=one\n\nChange-Id: Ixx'), + ('foo\n\nChange-Id: Ixx', 'R=one', 'foo\n\nR=one\n\nChange-Id: Ixx'), + ('foo\n\nR=one\n\nChange-Id: Ixx', 'TBR=two', + 'foo\n\nR=one\nTBR=two\n\nChange-Id: Ixx'), + ('foo\n\nR=one\n\nChange-Id: Ixx', 'Foo-Bar: baz', + 'foo\n\nR=one\n\nChange-Id: Ixx\nFoo-Bar: baz'), + ('foo\n\nChange-Id: Ixx', 'Foo-Bak: baz', + 'foo\n\nChange-Id: Ixx\nFoo-Bak: baz'), + ('foo', 'Change-Id: Ixx', 'foo\n\nChange-Id: Ixx'), + ]: + desc = git_cl.ChangeDescription(init_desc) + desc.append_footer(footer_line) + self.assertEqual(desc.description, expected_desc) + def test_update_reviewers(self): data = [ ('foo', [], 'foo'), @@ -1444,7 +1463,7 @@ class TestGitCl(TestCase): self.assertEqual('hihi', ChangelistMock.desc) def test_description_appends_bug_line(self): - current_desc = 'Some\n\nChange-Id: xxx' + current_desc = 'Some.\n\nChange-Id: xxx' def RunEditor(desc, _, **kwargs): self.assertEquals( @@ -1452,15 +1471,14 @@ class TestGitCl(TestCase): '# This will be displayed on the codereview site.\n' '# The first line will also be used as the subject of the review.\n' '#--------------------This line is 72 characters long' - '--------------------\n' + - # TODO(tandrii): fix this http://crbug.com/614587. - current_desc + '\n\nBUG=', + '--------------------\n' + 'Some.\n\nBUG=\n\nChange-Id: xxx', desc) - return current_desc + '\n\nBUG=' + # Simulate user changing something. + return 'Some.\n\nBUG=123\n\nChange-Id: xxx' def UpdateDescriptionRemote(_, desc): - # TODO(tandrii): fix this http://crbug.com/614587. - self.assertEquals(desc, current_desc + '\n\nBUG=') + self.assertEquals(desc, 'Some.\n\nBUG=123\n\nChange-Id: xxx') self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.mock(git_cl.Changelist, 'GetDescription',