From 562c59c76c30989e7cdfad64310b0cefd1bb1f99 Mon Sep 17 00:00:00 2001
From: Scott Lee <ddoman@chromium.org>
Date: Tue, 3 Dec 2024 17:24:52 +0000
Subject: [PATCH] [git_footers] add support for multiline footers

Gerrit allows multiline footers with indents.

e.g.,
Test: something
   looks great

This CL fixes the bug such that add_footer correctly inserts
a given message after multiline footers.

Bug: 379923433
Change-Id: I9b3f793095b63b0586123543a2f8d49f0503fca0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6064963
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Scott Lee <ddoman@chromium.org>
---
 git_footers.py            | 38 +++++++++-------
 tests/git_footers_test.py | 94 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+), 15 deletions(-)

diff --git a/git_footers.py b/git_footers.py
index 07c6feb19..0426ead09 100755
--- a/git_footers.py
+++ b/git_footers.py
@@ -42,15 +42,12 @@ def parse_footers(message):
     return footer_map
 
 
-def matches_footer_key(line, key):
-    """Returns whether line is a valid footer whose key matches a given one.
-
-    Keys are compared in normalized form.
-    """
+def normalize_key(line):
+    """Returns the key of the footer line in normalized form."""
     r = parse_footer(line)
     if r is None:
-        return False
-    return normalize_name(r[0]) == normalize_name(key)
+        return ''
+    return normalize_name(r[0])
 
 
 def split_footers(message):
@@ -140,16 +137,27 @@ def add_footer(message, key, value, after_keys=None):
             top_lines.append('')
         footer_lines = [new_footer]
     else:
+        # find the index of the last footer with any of the after_keys.
         after_keys = set(map(normalize_name, after_keys or []))
-        after_indices = [
-            footer_lines.index(x) for x in footer_lines for k in after_keys
-            if matches_footer_key(x, k)
-        ]
-        if after_indices:
-            # after_keys takes precedence, even if there's a conflict.
-            insert_idx = max(after_indices) + 1
-        else:
+        last_akey_idx = None
+        for idx in range(len(footer_lines)):
+            line = footer_lines[idx]
+
+            # if the current footer is indented, it may be a continuation of
+            # the previous line.
+            if line and line[0] == ' ':
+                if last_akey_idx is None:
+                    continue
+                if idx - last_akey_idx > 1:
+                    continue
+            elif normalize_key(line) not in after_keys:
+                continue
+            last_akey_idx = idx
+
+        if last_akey_idx is None:
             insert_idx = len(footer_lines)
+        else:
+            insert_idx = last_akey_idx + 1
         footer_lines.insert(insert_idx, new_footer)
     return '\n'.join(top_lines + footer_lines)
 
diff --git a/tests/git_footers_test.py b/tests/git_footers_test.py
index 2fd0aee66..d1a3999a2 100755
--- a/tests/git_footers_test.py
+++ b/tests/git_footers_test.py
@@ -171,6 +171,100 @@ My commit message is my best friend. It is my life.
                                              'Ix'),
             'Header.\n\nBug: v8\nChange-Id: Ix\nN=t\nT=z')
 
+    def testAddFooterChangeIdWithMultilineFooters(self):
+        add_change_id = lambda lines: git_footers.add_footer_change_id(
+            '\n'.join(lines), 'Ixxx')
+
+        self.assertEqual(
+            add_change_id([
+                'header',
+                '',
+                '',
+                'BUG: yy',
+                'Test: hello ',
+                '  world',
+            ]),
+            '\n'.join([
+                'header',
+                '',
+                '',
+                'BUG: yy',
+                'Test: hello ',
+                '  world',
+                'Change-Id: Ixxx',
+            ]),
+        )
+
+        self.assertEqual(
+            add_change_id([
+                'header',
+                '',
+                '',
+                'BUG: yy',
+                'Yeah: hello ',
+                '  world',
+            ]),
+            '\n'.join([
+                'header',
+                '',
+                '',
+                'BUG: yy',
+                'Change-Id: Ixxx',
+                'Yeah: hello ',
+                '  world',
+            ]),
+        )
+
+        self.assertEqual(
+            add_change_id([
+                'header',
+                '',
+                '',
+                'Something: ',
+                '   looks great',
+                'BUG: yy',
+                'Test: hello ',
+                '  world',
+            ]),
+            '\n'.join([
+                'header',
+                '',
+                '',
+                'Something: ',
+                '   looks great',
+                'BUG: yy',
+                'Test: hello ',
+                '  world',
+                'Change-Id: Ixxx',
+            ]),
+        )
+
+        self.assertEqual(
+            add_change_id([
+                'header',
+                '',
+                '',
+                'Something: ',
+                'BUG: yy',
+                'Something: ',
+                '   looks great',
+                'Test: hello ',
+                '  world',
+            ]),
+            '\n'.join([
+                'header',
+                '',
+                '',
+                'Something: ',
+                'BUG: yy',
+                'Something: ',
+                '   looks great',
+                'Test: hello ',
+                '  world',
+                'Change-Id: Ixxx',
+            ]),
+        )
+
     def testAddFooter(self):
         with self.assertRaises(ValueError):
             git_footers.add_footer('', 'Invalid Footer', 'Value')