bug#63378: [PATCH] teams: Fix script to produce a single X-Debbugs-Cc header.

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hi Arun,

Arun Isaac <arunisaac@systemreboot.net> writes:

> Hi Maxim,
>
> When a patch relates to no teams, I get the empty header output
> "X-Debbugs-Cc: ". For such patches, it would be nicer to not add any
> header instead of adding an empty header.

Good catch!  Fixed.

>> +(define (team->members team)
>> +  "Return the list of members in TEAM."
>> +  (sort (team-members team)
>> +        (lambda (m1 m2)
>> +          (string<? (person-name m1) (person-name m2)))))
>> +
>> +(define (member->string member)
>> +  "Return the 'email <name>' string representation of MEMBER."
>> +  (format #false "~a <~a>" (person-name member) (person-email
>> member)))
>
> When person name contains a comma, it should be escaped by surrounding
> in double quotes. Else, the comma would interfere with the
> comma-separated list that we are constructing for
> X-Debbugs-Cc. Admittedly, none of our current team members have commas
> in their person names. But, some people like to have a name like
> "LastName, FirstName". We should protect against that edge case in the
> interest of futureproofing.

OK!  I opted for simplicity and double-quoted all the names.

>> +     (format #true "X-Debbugs-Cc: ~{~a~^,~}"
>> +             (append-map (compose (cut map member->string <>)
>> +                                  team->members
>> +                                  find-team)
>> +                         (patch->teams patch-file))))
>
> A very nitpicky suggestion: Maybe, separate multiple persons by ", "
> instead of just ",". ", " looks slightly better cosmetically. Likewise
> in other places where this occurs.

Good idea; I've learnt this was valid email syntax just today :-).

Below is the diff of my rework:

--8<---------------cut here---------------start------------->8---
1 file changed, 18 insertions(+), 17 deletions(-)
etc/teams.scm.in | 35 ++++++++++++++++++-----------------

modified   etc/teams.scm.in
@@ -605,20 +605,20 @@ (define (find-team-by-scope files)
 (define (cc . teams)
   "Return arguments for `git send-email' to notify the members of the given
 TEAMS when a patch is received by Debbugs."
-  (format #true
-          "--add-header=\"X-Debbugs-Cc: ~{~a~^,~}\""
-          (map person-email
-               (delete-duplicates (append-map team-members teams) equal?))))
-
-(define (team->members team)
-  "Return the list of members in TEAM."
-  (sort (team-members team)
+  (let ((members (append-map team-members teams)))
+    (unless (null? members)
+      (format #true "--add-header=\"X-Debbugs-Cc: ~{~a~^, ~}\""
+              (map person-email (sort-members members))))))
+
+(define (sort-members members)
+  "Deduplicate and sort MEMBERS alphabetically by their name."
+  (sort (delete-duplicates members equal?)
         (lambda (m1 m2)
           (string<? (person-name m1) (person-name m2)))))
 
 (define (member->string member)
   "Return the 'email <name>' string representation of MEMBER."
-  (format #false "~a <~a>" (person-name member) (person-email member)))
+  (format #false "\"~a\" <~a>" (person-name member) (person-email member)))
 
 (define* (list-members team #:optional port (prefix ""))
   "Print the members of the given TEAM."
@@ -626,7 +626,7 @@ (define* (list-members team #:optional port (prefix ""))
   (for-each
    (lambda (member)
      (format port* "~a~a~%" prefix (member->string member)))
-   (team->members team)))
+   (sort-members (team-members team))))
 
 (define (list-teams)
   "Print all teams, their scope and their members."
@@ -720,14 +720,15 @@ (define (main . args)
      (apply cc (find-team-by-scope
                 (diff-revisions rev-start rev-end))))
     (("cc-members-header-cmd" patch-file)
-     (format #true "X-Debbugs-Cc: ~{~a~^,~}"
-             (append-map (compose (cut map member->string <>)
-                                  team->members
-                                  find-team)
-                         (patch->teams patch-file))))
+     (let* ((teams (map find-team (patch->teams patch-file)))
+            (members (sort-members (append-map team-members teams))))
+       (unless (null? members)
+         (format #true "X-Debbugs-Cc: ~{~a~^, ~}"
+                 (map member->string members)))))
     (("cc-mentors-header-cmd" patch-file)
-     (format #true "X-Debbugs-Cc: ~{~a~^,~}"
-             (map member->string (team->members (find-team "mentors")))))
+     (format #true "X-Debbugs-Cc: ~{~a~^, ~}"
+             (map member->string
+                  (sort-members (team-members (find-team "mentors"))))))
     (("get-maintainer" patch-file)
      (apply main "list-members" (patch->teams patch-file)))
     (("list-teams" . args)
--8<---------------cut here---------------end--------------->8---

A proper v2 patch has been sent.

-- 
Thanks,
Maxim