bug#60147: 30.0.50; vc-prepare-patch: Add numbered patch file names

[Philip Kaludercic]

Sean Whitton <spwhitton@spwhitton.name> writes:

> X-debbugs-cc: philipk@posteo.net
>
> Hello,
>
> If you receive a message generated by vc-prepare-patch and then you save all
> the attachments to a directory, you lose any indication of the order in which
> the patches are meant to be applied.
>
> git-format-patch(1) numbers the patch files it saves.  Thus, with most mail
> clients, when the recipient saves all the attachments to a local directory,
> they'll be ordered.  I think it would be useful for vc-prepare-patch to do
> something similar.  Please find a patch attached.

This sounds good, find my comments below.

> --
> Sean Whitton
>
> From 226e689b1183a34f3b611185201416e4f3205304 Mon Sep 17 00:00:00 2001
> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Fri, 16 Dec 2022 22:34:52 -0700
> Subject: [PATCH] vc-prepare-patch: Number the attached patches
>
> * lisp/gnus/mml.el (mml-attach-buffer): New FILENAME argument.
> * lisp/vc/vc.el (vc-prepare-patch): When vc-prepare-patches-separately
> is nil, generate file names for the attached patches.  The file names
> begin with numbers indicating the ordering of the patches.
> ---
>  lisp/gnus/mml.el | 13 ++++++++-----
>  lisp/vc/vc.el    | 26 +++++++++++++++++++++-----
>  2 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/lisp/gnus/mml.el b/lisp/gnus/mml.el
> index ebd0adf2e25..dc86fe6db96 100644
> --- a/lisp/gnus/mml.el
> +++ b/lisp/gnus/mml.el
> @@ -1484,10 +1484,12 @@ mml-dnd-attach-file
>         (setq disposition (mml-minibuffer-read-disposition type nil file)))
>       (mml-attach-file file type description disposition)))))
>  
> -(defun mml-attach-buffer (buffer &optional type description disposition)
> +(defun mml-attach-buffer (buffer &optional type description disposition 
> filename)
>    "Attach a buffer to the outgoing MIME message.
>  BUFFER is the name of the buffer to attach.  See
> -`mml-attach-file' for details of operation."
> +`mml-attach-file' regarding TYPE, DESCRIPTION and DISPOSITION.
> +FILENAME is a suggested file name for the attachment should a
> +recipient wish to save a copy separate from the message."
>    (interactive
>     (let* ((buffer (read-buffer "Attach buffer: "))
>         (type (mml-minibuffer-read-type buffer "text/plain"))
> @@ -1497,9 +1499,10 @@ mml-attach-buffer
>    ;; If in the message header, attach at the end and leave point unchanged.
>    (let ((head (unless (message-in-body-p) (point))))
>      (if head (goto-char (point-max)))
> -    (mml-insert-empty-tag 'part 'type type 'buffer buffer
> -                       'disposition disposition
> -                       'description description)
> +    (apply #'mml-insert-empty-tag
> +           'part 'type type 'buffer buffer
> +        'disposition disposition 'description description
> +           (and filename `(filename ,filename)))
>      ;; When using Mail mode, make sure it does the mime encoding
>      ;; when you send the message.
>      (or (eq mail-user-agent 'message-user-agent)
> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
> index 690c907c77e..ed10909fee8 100644
> --- a/lisp/vc/vc.el
> +++ b/lisp/vc/vc.el
> @@ -3466,11 +3466,27 @@ vc-prepare-patch
>          (rfc822-goto-eoh)
>          (forward-line)
>          (save-excursion
> -          (dolist (patch patches)
> -            (mml-attach-buffer (buffer-name (plist-get patch :buffer))
> -                               "text/x-patch"
> -                               (plist-get patch :subject)
> -                               "attachment")))
> +          (let ((i 0))
> +            (dolist (patch patches)
> +              (let* ((patch-subject (plist-get patch :subject))
> +                     (stripped-subject
> +                      (replace-regexp-in-string
> +                       "^\\[.*PATCH.*\\]\\s-*" "" patch-subject))

I think using a \` instead of ^ would be more robust.

> +                     (filename
> +                      (concat
> +                       (string-trim
> +                        (replace-regexp-in-string
> +                         "\\W" "-" (if (length> stripped-subject 50)
> +                                       (substring stripped-subject 0 50)
> +                                     stripped-subject))

Is limiting the file names to ~50 characters a Git thing?

> +                        "-+" "-+")
> +                       ".patch")))

Perhaps this should be pulled out into a separate function.

> +                (mml-attach-buffer
> +                 (buffer-name (plist-get patch :buffer))
> +                 "text/x-patch"
> +                 patch-subject
> +                 "attachment"
> +                 (format "%04d-%s" (cl-incf i) filename))))))

Is the new additional argument really necessary, or couldn't we just
rename the generated buffer?  We could specify that the buffer must be
fresh/renameable.

>          (open-line 2)))))
>  
>  (defun vc-default-responsible-p (_backend _file)