Subject: Re: bug#57400: 29.0.50; Support sending patches from VC directly

From: Robert Pluim <[email-masked]>
Date: Thu, 06 Oct 2022 13:33:04 +0200

>>>>> On Wed, 05 Oct 2022 17:34:22 +0000, Philip Kaludercic <[email-masked]> said:

    Philip> +@code{vc-prepare-patch} command.  This will prompt you which revisions
    Philip> +you wish to share and who the addressee is.  The command will then use
    Philip> +your @abbr{MUA, Mail User Agent} for you to review and send out.
    Philip> +

How about

--begin--
This will prompt you for the revisions you wish to share, and which
destination email address(es) to use.  The command will then prepare
those revisions using your @abbr{MUA, Mail User Agent} for you to
review and send.
--end--

The semantics is 'one-or-more addresses', right?

    Philip> +@vindex vc-prepare-patches-inline
    Philip> +Depending on configuration of the user option

"Depending on the value of the user option"

    Philip> +@code{vc-prepare-patches-inline}, @code{vc-prepare-patch} will either
    Philip> +generate a single or multiple messages.  A @code{nil} value (the default)
    Philip> +will prepare and display a message for each revision, one after
    Philip> +another.  A non-@code{nil} value will have all patches attached to the
    Philip> +body of a single message.
    Philip> +

--begin--
@code{vc-prepare-patches-inline}, @code{vc-prepare-patch} will
generate one or more messages.  The default value @code{nil} means
prepare and display a message for each revision, one after another.  A
non-@code{nil} value means to generate a single message with all
patches attached in the body.
--end--

    Philip> +@vindex vc-default-patch-addressee
    Philip> +If you expect to contribute patches on a regular basis, you can set
    Philip> +the user option @code{vc-default-patch-addressee} to the address you
    Philip> +wish to use.  This will be used as the default value when invoking
    Philip> +@code{vc-prepare-patch}.  Project maintainers may consider setting
    Philip> +this as a directory local variable (@pxref{Directory Variables}).
    Philip> +

This can contain multiple addresses, I think, in which case it should
say so.

    Philip> +** Subr-x
    Philip> +
    Philip> +---
    Philip> +*** New macro 'with-funcall-substitutions'.
    Philip> +The macro can be used to generically substitute function symbols in
    Philip> +expressions.
    Philip> +
    Philip>  ** Ansi-color

That sounds interesting, but I donʼt see it in the patch :-)
 
    Philip> +(defcustom vc-prepare-patches-inline nil
    Philip> +  "Non-nil means that `vc-prepare-patch' creates a single
    Philip> message.

"Whether `vc-prepare-patch' attaches all revision in a single message."

Iʼm not sure this should have the suffix '-inline', because you can
have inline attachments and attached attachments, but itʼs not a big
deal.

I also wonder about the default. Creating 100 mail buffers by accident
is harder to recover from than a single one with 100 attachments, but
I guess experience will inform us.

    Philip> +A single message is created by attaching all patches to the body
    Philip> +of a single message.  If nil, each patch will be sent out in a
    Philip> +separate message, which will be prepared sequentially."
    Philip> +  :type 'boolean
    Philip> +  :safe #'booleanp
    Philip> +  :version "29.1")
    Philip> +

(I didnʼt check, can this do the [PATCH n/m] stuff with the
subject that 'git format-patch' can do?)

    Philip> +(defcustom vc-default-patch-addressee nil
    Philip> +  "Default addressee for `vc-prepare-patch'.
    Philip> +If nil, no default will be used.  This option may be set locally."
    Philip> +  :type '(choice (const :tag "No default" nil) string)
    Philip> +  :safe #'stringp
    Philip> +  :version "29.1")
    Philip> +

Again, I think this can be multiple addresses. Either as a string
with commas or as a list of strings perhaps? 

    Philip> +;;;###autoload
    Philip> +(defun vc-prepare-patch (addressee subject revisions)
    Philip> +  "Compose an Email sending patches for REVISIONS to ADDRESSEE.
    Philip> +If `vc-prepare-patches-inline' is non-nil, SUBJECT will be used
    Philip> +as the default subject for the message.  Otherwise a separate
    Philip> +message will be composed for each revision.
    Philip> +

? What does `vc-prepare-patches-inline' have to do with the SUBJECT?

    Philip> It includes

    Philip> - some documentation for the Emacs manual and etc/NEWS,

    Philip> - a revised "prepare-patch" interface that uses buffers instead of
    Philip>   temporary files (I hope this improves the encoding issue),

If itʼs all buffers now then I think you need to update this comment:

+;;
+;; - prepare-patch (rev)
+;;
+;;   Prepare a patch and return a property list with the keys
+;;   `:subject' indicating the patch message as a string, `:body'
+;;   containing the contents of the patch as a string (excluding the
+;;   header) and `:filename' pointing to a file where the patch has
+;;   been stored.

I have no firm opinion on if there should be a default binding nor
what it should be 😺

Thanks for this, it will be useful

Robert
--