Preserve trailer order by default

Per discussion on the users list, preserve the trailer order by default.
There is no agreement on whether this is a hard requirement for patches
or not, but there is general consensus that the default should be to
make as few changes to incoming patches as possible.

Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

3 files changed, 38 insertions, 31 deletions

diff --git a/b4/__init__.py b/b4/__init__.py
index 4325121..2f6d1ba 100644
--- a/b4/__init__.py
+++ b/b4/__init__.py
@@ -66,7 +66,12 @@ WANTHDRS = [
 # [b4]
 #   # remember to end with ,*
 #   trailer-order=link*,fixes*,cc*,reported*,suggested*,original*,co-*,tested*,reviewed*,acked*,signed-off*,*
-DEFAULT_TRAILER_ORDER = 'fixes*,reported*,suggested*,original*,co-*,signed-off*,tested*,reviewed*,acked*,cc*,link*,*'
+#   (another common)
+#   trailer-order=fixes*,reported*,suggested*,original*,co-*,signed-off*,tested*,reviewed*,acked*,cc*,link*,*
+#
+# Or use _preserve_ (alias to *) to keep the order unchanged
+
+DEFAULT_TRAILER_ORDER = '*'
 
 LOREADDR = 'https://lore.kernel.org'
 
@@ -300,17 +305,17 @@ class LoreMailbox:
                         pmsg.load_hashes()
                         attid = pmsg.attestation.attid
                         if attid not in self.trailer_map:
-                            self.trailer_map[attid] = set()
-                        self.trailer_map[attid].update(trailers)
-                    pmsg.followup_trailers.update(trailers)
+                            self.trailer_map[attid] = list()
+                        self.trailer_map[attid] += trailers
+                    pmsg.followup_trailers += trailers
                     break
                 if not pmsg.reply:
                     # Could be a cover letter
-                    pmsg.followup_trailers.update(trailers)
+                    pmsg.followup_trailers += trailers
                     break
                 if pmsg.in_reply_to and pmsg.in_reply_to in self.msgid_map:
                     lvl += 1
-                    trailers.update(pmsg.trailers)
+                    trailers += pmsg.trailers
                     pmsg = self.msgid_map[pmsg.in_reply_to]
                     continue
                 break
@@ -321,7 +326,7 @@ class LoreMailbox:
                 continue
             lmsg.load_hashes()
             if lmsg.attestation.attid in self.trailer_map:
-                lmsg.followup_trailers.update(self.trailer_map[lmsg.attestation.attid])
+                lmsg.followup_trailers += self.trailer_map[lmsg.attestation.attid]
 
         return lser
 
@@ -505,11 +510,11 @@ class LoreSeries:
                 continue
             if lmsg is not None:
                 if self.has_cover and covertrailers and self.patches[0].followup_trailers:
-                    lmsg.followup_trailers.update(self.patches[0].followup_trailers)
+                    lmsg.followup_trailers += self.patches[0].followup_trailers
                 if addmysob:
-                    lmsg.followup_trailers.add(('Signed-off-by', '%s <%s>' % (usercfg['name'], usercfg['email'])))
+                    lmsg.followup_trailers.append(('Signed-off-by', '%s <%s>' % (usercfg['name'], usercfg['email'])))
                 if addlink:
-                    lmsg.followup_trailers.add(('Link', linkmask % lmsg.msgid))
+                    lmsg.followup_trailers.append(('Link', linkmask % lmsg.msgid))
 
                 if attpolicy != 'off':
                     lore_lookup = False
@@ -780,8 +785,8 @@ class LoreMessage:
         self.charset = 'utf-8'
         self.has_diff = False
         self.has_diffstat = False
-        self.trailers = set()
-        self.followup_trailers = set()
+        self.trailers = list()
+        self.followup_trailers = list()
 
         # These are populated by pr
         self.pr_base_commit = None
@@ -879,7 +884,7 @@ class LoreMessage:
             matches = re.findall(r'^\s*Fixes:[ \t]+([a-f0-9]+\s+\(.*\))\s*$', self.body, re.MULTILINE)
             if matches:
                 for tvalue in matches:
-                    self.trailers.add(('Fixes', tvalue))
+                    self.trailers.append(('Fixes', tvalue))
 
             # Do we have something that looks like a person-trailer?
             matches = re.findall(r'^\s*([\w-]{2,}):[ \t]+(.*<\S+>)\s*$', self.body, re.MULTILINE)
@@ -888,17 +893,17 @@ class LoreMessage:
             if matches:
                 for tname, tvalue in matches:
                     if tname.lower() not in badtrailers:
-                        self.trailers.add((tname, tvalue))
+                        self.trailers.append((tname, tvalue))
 
     def get_trailers(self, sloppy=False):
         mismatches = set()
         if sloppy:
             return set(self.trailers), mismatches
 
-        trailers = set()
+        trailers = list()
         for tname, tvalue in self.trailers:
             if tname.lower() in ('fixes',):
-                trailers.add((tname, tvalue))
+                trailers.append((tname, tvalue))
                 continue
 
             tmatch = False
@@ -935,7 +940,7 @@ class LoreMessage:
                     logger.debug('  trailer fuzzy name match')
                     tmatch = True
             if tmatch:
-                trailers.add((tname, tvalue))
+                trailers.append((tname, tvalue))
             else:
                 mismatches.add((tname, tvalue))
 
@@ -1263,10 +1268,12 @@ class LoreMessage:
     def fix_trailers(self, trailer_order=None):
         bheaders, message, btrailers, basement, signature = LoreMessage.get_body_parts(self.body)
         # Now we add mix-in trailers
-        trailers = btrailers + list(self.followup_trailers)
+        trailers = btrailers + self.followup_trailers
         fixtrailers = list()
         if trailer_order is None:
             trailer_order = DEFAULT_TRAILER_ORDER
+        elif trailer_order in ('preserve', '_preserve_'):
+            trailer_order = '*'
         for trailermatch in trailer_order:
             for trailer in trailers:
                 if trailer in fixtrailers:
diff --git a/man/b4.5 b/man/b4.5
index 95876b8..cdbd1d0 100644
--- a/man/b4.5
+++ b/man/b4.5
@@ -316,8 +316,6 @@ Save diff into this file instead of outputting to stdout
 .B \-c\fP,\fB  \-\-color
 Force color output even when writing to file
 .UNINDENT
-.IP "System Message: WARNING/2 (b4.5.rst:, line 201)"
-Option list ends without a blank line; unexpected unindent.
 .INDENT 0.0
 .TP
 .B \-m AMBOX AMBOX, \-\-compare\-am\-mboxes AMBOX AMBOX
@@ -326,7 +324,7 @@ Compare two mbx files prepared with "b4 am"
 .UNINDENT
 .UNINDENT
 .sp
-\fIExample\fP: b4 diff
+\fIExample\fP: b4 diff \fI\%20200526205322.23465\-1\-mic@digikod.net\fP
 .SH CONFIGURATION
 .sp
 B4 configuration is handled via git\-config(1), so you can store it in
@@ -344,13 +342,14 @@ Default configuration, with explanations:
    midmask = https://lore.kernel.org/r/%s\(aq
    #
    # When recording Link: trailers, use this mask
-   linkmask = https://lore.kernel.org/r/%s\(aq
+   linkmask = https://lore.kernel.org/r/%s
    #
-   # When processing thread trailers, use this order. Can use shell\-globbing
-   # and must end with ,*
-   # Common alternative order:
+   # When processing thread trailers, sort them in this order.
+   # Can use shell\-globbing and must end with ,*
+   # Some sorting orders:
    #trailer\-order=link*,fixes*,cc*,reported*,suggested*,original*,co\-*,tested*,reviewed*,acked*,signed\-off*,*
-   trailer\-order = fixes*,reported*,suggested*,original*,co\-*,signed\-off*,tested*,reviewed*,acked*,cc*,link*,*
+   #trailer\-order = fixes*,reported*,suggested*,original*,co\-*,signed\-off*,tested*,reviewed*,acked*,cc*,link*,*
+   trailer\-order = _preserve_
    #
    # Attestation\-checking configuration parameters
    # off: do not bother checking attestation
@@ -389,7 +388,7 @@ Default configuration, with explanations:
    # How long to keep downloaded threads in cache (minutes)?
    cache\-expire = 10
    # Used when creating summaries for b4 ty, and can be set to a value like
-   # thanks\-commit\-url\-mask = https://git.kernel.org/username/c/%.10s
+   # thanks\-commit\-url\-mask = https://git.kernel.org/username/c/%.12s
    # See this page for more info on convenient git.kernel.org shorterners:
    # https://korg.wiki.kernel.org/userdoc/git\-url\-shorterners
    thanks\-commit\-url\-mask = None
diff --git a/man/b4.5.rst b/man/b4.5.rst
index 2591a04..9e0d995 100644
--- a/man/b4.5.rst
+++ b/man/b4.5.rst
@@ -219,11 +219,12 @@ Default configuration, with explanations::
       # When recording Link: trailers, use this mask
       linkmask = https://lore.kernel.org/r/%s
       #
-      # When processing thread trailers, use this order. Can use shell-globbing
-      # and must end with ,*
-      # Common alternative order:
+      # When processing thread trailers, sort them in this order.
+      # Can use shell-globbing and must end with ,*
+      # Some sorting orders:
       #trailer-order=link*,fixes*,cc*,reported*,suggested*,original*,co-*,tested*,reviewed*,acked*,signed-off*,*
-      trailer-order = fixes*,reported*,suggested*,original*,co-*,signed-off*,tested*,reviewed*,acked*,cc*,link*,*
+      #trailer-order = fixes*,reported*,suggested*,original*,co-*,signed-off*,tested*,reviewed*,acked*,cc*,link*,*
+      trailer-order = _preserve_
       #
       # Attestation-checking configuration parameters
       # off: do not bother checking attestation