all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#60147: 30.0.50; vc-prepare-patch: Add numbered patch file names
@ 2022-12-17  5:40 Sean Whitton
  2022-12-17  9:33 ` Philip Kaludercic
  0 siblings, 1 reply; 5+ messages in thread
From: Sean Whitton @ 2022-12-17  5:40 UTC (permalink / raw)
  To: 60147; +Cc: philipk

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

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.

--
Sean Whitton

[-- Attachment #2: 0001-vc-prepare-patch-Number-the-attached-patches.patch --]
[-- Type: text/x-diff, Size: 3767 bytes --]

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))
+                     (filename
+                      (concat
+                       (string-trim
+                        (replace-regexp-in-string
+                         "\\W" "-" (if (length> stripped-subject 50)
+                                       (substring stripped-subject 0 50)
+                                     stripped-subject))
+                        "-+" "-+")
+                       ".patch")))
+                (mml-attach-buffer
+                 (buffer-name (plist-get patch :buffer))
+                 "text/x-patch"
+                 patch-subject
+                 "attachment"
+                 (format "%04d-%s" (cl-incf i) filename))))))
         (open-line 2)))))
 
 (defun vc-default-responsible-p (_backend _file)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#60147: 30.0.50; vc-prepare-patch: Add numbered patch file names
  2022-12-17  5:40 bug#60147: 30.0.50; vc-prepare-patch: Add numbered patch file names Sean Whitton
@ 2022-12-17  9:33 ` Philip Kaludercic
  2022-12-18  0:33   ` Sean Whitton
  0 siblings, 1 reply; 5+ messages in thread
From: Philip Kaludercic @ 2022-12-17  9:33 UTC (permalink / raw)
  To: Sean Whitton; +Cc: 60147

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)





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#60147: 30.0.50; vc-prepare-patch: Add numbered patch file names
  2022-12-17  9:33 ` Philip Kaludercic
@ 2022-12-18  0:33   ` Sean Whitton
  2022-12-18 10:45     ` Philip Kaludercic
  0 siblings, 1 reply; 5+ messages in thread
From: Sean Whitton @ 2022-12-18  0:33 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: 60147

Hello,

On Sat 17 Dec 2022 at 09:33AM GMT, Philip Kaludercic wrote:

>> +                     (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?

Git does it, yes, and I thought it seemed like a good idea in general.

>> +                (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.

The description and the filename for an attachment are not the same
thing -- I don't believe MUAs will save the files with the correct name
unless there is the filename= field.  And I think it's a useful general
addition to mml-attach-buffer.

-- 
Sean Whitton





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#60147: 30.0.50; vc-prepare-patch: Add numbered patch file names
  2022-12-18  0:33   ` Sean Whitton
@ 2022-12-18 10:45     ` Philip Kaludercic
  2022-12-20  0:17       ` Sean Whitton
  0 siblings, 1 reply; 5+ messages in thread
From: Philip Kaludercic @ 2022-12-18 10:45 UTC (permalink / raw)
  To: Sean Whitton; +Cc: 60147

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,
>
> On Sat 17 Dec 2022 at 09:33AM GMT, Philip Kaludercic wrote:
>
>>> +                     (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?
>
> Git does it, yes, and I thought it seemed like a good idea in general.

OK, then it should be a good convention.  And if the file names are
numbered, then there shouldn't be any conflicts when downloading
patches.

>>> +                (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.
>
> The description and the filename for an attachment are not the same
> thing -- I don't believe MUAs will save the files with the correct name
> unless there is the filename= field.  And I think it's a useful general
> addition to mml-attach-buffer.

I see, in that case this should be fine.  Can you apply the patch?





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#60147: 30.0.50; vc-prepare-patch: Add numbered patch file names
  2022-12-18 10:45     ` Philip Kaludercic
@ 2022-12-20  0:17       ` Sean Whitton
  0 siblings, 0 replies; 5+ messages in thread
From: Sean Whitton @ 2022-12-20  0:17 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: 60147-done

Hello,

Pushed.  Thank you for looking.  We may later want to refine the
vc--subject-to-file-name subroutine to replace other characters too.

-- 
Sean Whitton





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-12-20  0:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-17  5:40 bug#60147: 30.0.50; vc-prepare-patch: Add numbered patch file names Sean Whitton
2022-12-17  9:33 ` Philip Kaludercic
2022-12-18  0:33   ` Sean Whitton
2022-12-18 10:45     ` Philip Kaludercic
2022-12-20  0:17       ` Sean Whitton

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.