mail-add-attachment

mail-add-attachment

[Eli Zaretskii]

Is it okay to install this command at this time?  I know we're in a
feature freeze, but the new command is actually a very thin wrapper
around code we already use elsewhere (gnus-dired.el and mml.el).  The
result will be that users of sendmail.el will be able to send MIME
attachments.  Here's the patch:

=== modified file 'lisp/mail/sendmail.el'
--- lisp/mail/sendmail.el	2011-09-03 10:00:13 +0000
+++ lisp/mail/sendmail.el	2011-09-03 10:00:32 +0000
@@ -34,6 +34,21 @@
 (autoload 'mml-to-mime "mml"
   "Translate the current buffer from MML to MIME.")
 
+(autoload 'mml-attach-file "mml"
+  "Attach a file to the outgoing MIME message.
+The file is not inserted or encoded until you send the message with
+`\\[message-send-and-exit]' or `\\[message-send]'.
+
+FILE is the name of the file to attach.  TYPE is its
+content-type, a string of the form \"type/subtype\".  DESCRIPTION
+is a one-line description of the attachment.  The DISPOSITION
+specifies how the attachment is intended to be displayed.  It can
+be either \"inline\" (displayed automatically within the message
+body) or \"attachment\" (separate from the body).")
+
+(autoload 'mm-default-file-encoding "mm-encode"
+  "Return the default encoding for FILE.")
+
 (defgroup sendmail nil
   "Mail sending commands for Emacs."
   :prefix "mail-"
@@ -355,6 +370,9 @@ The default value matches citations like
     (define-key map [menu-bar mail]
       (cons "Mail" (make-sparse-keymap "Mail")))
 
+    (define-key map [menu-bar mail attachment]
+      '("Attach File" . mail-add-attachment))
+
     (define-key map [menu-bar mail fill]
       '("Fill Citation" . mail-fill-yanked-message))
 
@@ -700,6 +718,8 @@ Here are commands that move to a header 
 \\[mail-signature]  mail-signature (insert `mail-signature-file' file).
 \\[mail-yank-original]  mail-yank-original (insert current message, in Rmail).
 \\[mail-fill-yanked-message]  mail-fill-yanked-message (fill what was yanked).
+\\[mail-attach-file] insert a text file into the message.
+\\[mail-add-attachment] add to the message a file as a MIME attachment.
 Turning on Mail mode runs the normal hooks `text-mode-hook' and
 `mail-mode-hook' (in that order)."
   (make-local-variable 'mail-reply-action)
@@ -1716,6 +1736,15 @@ If the current line has `mail-yank-prefi
       (insert-file-contents file)
       (or (bolp) (newline))
       (goto-char start))))
+
+(defun mail-add-attachment (file)
+  "Add a file as a MIME attachment to the end of the message."
+  (interactive "fAttach file: ")
+  (mml-attach-file file
+		   (or (mm-default-file-encoding file)
+		       "application/octet-stream") nil)
+  (setq mail-encode-mml t))
+
 \f
 ;; Put these commands last, to reduce chance of lossage from quitting
 ;; in middle of loading the file.

Re: mail-add-attachment

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> +(autoload 'mml-attach-file "mml"
> +  "Attach a file to the outgoing MIME message.
> +The file is not inserted or encoded until you send the message with
> +`\\[message-send-and-exit]' or `\\[message-send]'.
> +
> +FILE is the name of the file to attach.  TYPE is its
> +content-type, a string of the form \"type/subtype\".  DESCRIPTION
> +is a one-line description of the attachment.  The DISPOSITION
> +specifies how the attachment is intended to be displayed.  It can
> +be either \"inline\" (displayed automatically within the message
> +body) or \"attachment\" (separate from the body).")
> +
> +(autoload 'mm-default-file-encoding "mm-encode"
> +  "Return the default encoding for FILE.")

Why not just adding autoload cookies instead of this duplication?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: mail-add-attachment

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: emacs-devel@gnu.org
> Date: Sat, 03 Sep 2011 12:33:48 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > +(autoload 'mml-attach-file "mml"
> > +  "Attach a file to the outgoing MIME message.
> > +The file is not inserted or encoded until you send the message with
> > +`\\[message-send-and-exit]' or `\\[message-send]'.
> > +
> > +FILE is the name of the file to attach.  TYPE is its
> > +content-type, a string of the form \"type/subtype\".  DESCRIPTION
> > +is a one-line description of the attachment.  The DISPOSITION
> > +specifies how the attachment is intended to be displayed.  It can
> > +be either \"inline\" (displayed automatically within the message
> > +body) or \"attachment\" (separate from the body).")
> > +
> > +(autoload 'mm-default-file-encoding "mm-encode"
> > +  "Return the default encoding for FILE.")
> 
> Why not just adding autoload cookies instead of this duplication?

Yep, can do.

Re: mail-add-attachment

[Eli Zaretskii]

> Date: Sat, 03 Sep 2011 14:38:11 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > From: Andreas Schwab <schwab@linux-m68k.org>
> > Cc: emacs-devel@gnu.org
> > Date: Sat, 03 Sep 2011 12:33:48 +0200
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > +(autoload 'mml-attach-file "mml"
> > > +  "Attach a file to the outgoing MIME message.
> > > +The file is not inserted or encoded until you send the message with
> > > +`\\[message-send-and-exit]' or `\\[message-send]'.
> > > +
> > > +FILE is the name of the file to attach.  TYPE is its
> > > +content-type, a string of the form \"type/subtype\".  DESCRIPTION
> > > +is a one-line description of the attachment.  The DISPOSITION
> > > +specifies how the attachment is intended to be displayed.  It can
> > > +be either \"inline\" (displayed automatically within the message
> > > +body) or \"attachment\" (separate from the body).")
> > > +
> > > +(autoload 'mm-default-file-encoding "mm-encode"
> > > +  "Return the default encoding for FILE.")
> > 
> > Why not just adding autoload cookies instead of this duplication?
> 
> Yep, can do.

Btw, do we have any guidelines when to add an autoload form and when
to load an autoload cookie?  Because it seems we do both...

Re: mail-add-attachment

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> Btw, do we have any guidelines when to add an autoload form and when
> to load an autoload cookie?  Because it seems we do both...

As long as the autoload cookie does not do anything undesirable when
loading the file again, I don't see any reason to prefer one to the
other.

Re: mail-add-attachment

[Eli Zaretskii]

Ping!

> Date: Sat, 03 Sep 2011 13:05:21 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Is it okay to install this command at this time?  I know we're in a
> feature freeze, but the new command is actually a very thin wrapper
> around code we already use elsewhere (gnus-dired.el and mml.el).  The
> result will be that users of sendmail.el will be able to send MIME
> attachments.  Here's the patch:
> 
> === modified file 'lisp/mail/sendmail.el'
> --- lisp/mail/sendmail.el	2011-09-03 10:00:13 +0000
> +++ lisp/mail/sendmail.el	2011-09-03 10:00:32 +0000
> @@ -34,6 +34,21 @@
>  (autoload 'mml-to-mime "mml"
>    "Translate the current buffer from MML to MIME.")
>  
> +(autoload 'mml-attach-file "mml"
> +  "Attach a file to the outgoing MIME message.
> +The file is not inserted or encoded until you send the message with
> +`\\[message-send-and-exit]' or `\\[message-send]'.
> +
> +FILE is the name of the file to attach.  TYPE is its
> +content-type, a string of the form \"type/subtype\".  DESCRIPTION
> +is a one-line description of the attachment.  The DISPOSITION
> +specifies how the attachment is intended to be displayed.  It can
> +be either \"inline\" (displayed automatically within the message
> +body) or \"attachment\" (separate from the body).")
> +
> +(autoload 'mm-default-file-encoding "mm-encode"
> +  "Return the default encoding for FILE.")
> +
>  (defgroup sendmail nil
>    "Mail sending commands for Emacs."
>    :prefix "mail-"
> @@ -355,6 +370,9 @@ The default value matches citations like
>      (define-key map [menu-bar mail]
>        (cons "Mail" (make-sparse-keymap "Mail")))
>  
> +    (define-key map [menu-bar mail attachment]
> +      '("Attach File" . mail-add-attachment))
> +
>      (define-key map [menu-bar mail fill]
>        '("Fill Citation" . mail-fill-yanked-message))
>  
> @@ -700,6 +718,8 @@ Here are commands that move to a header 
>  \\[mail-signature]  mail-signature (insert `mail-signature-file' file).
>  \\[mail-yank-original]  mail-yank-original (insert current message, in Rmail).
>  \\[mail-fill-yanked-message]  mail-fill-yanked-message (fill what was yanked).
> +\\[mail-attach-file] insert a text file into the message.
> +\\[mail-add-attachment] add to the message a file as a MIME attachment.
>  Turning on Mail mode runs the normal hooks `text-mode-hook' and
>  `mail-mode-hook' (in that order)."
>    (make-local-variable 'mail-reply-action)
> @@ -1716,6 +1736,15 @@ If the current line has `mail-yank-prefi
>        (insert-file-contents file)
>        (or (bolp) (newline))
>        (goto-char start))))
> +
> +(defun mail-add-attachment (file)
> +  "Add a file as a MIME attachment to the end of the message."
> +  (interactive "fAttach file: ")
> +  (mml-attach-file file
> +		   (or (mm-default-file-encoding file)
> +		       "application/octet-stream") nil)
> +  (setq mail-encode-mml t))
> +
>  \f
>  ;; Put these commands last, to reduce chance of lossage from quitting
>  ;; in middle of loading the file.
> 

Re: mail-add-attachment

[Stefan Monnier]

> Is it okay to install this command at this time?  I know we're in a
> feature freeze, but the new command is actually a very thin wrapper
> around code we already use elsewhere (gnus-dired.el and mml.el).  The
> result will be that users of sendmail.el will be able to send MIME
> attachments.  Here's the patch:

I guess it could be OK.
I have a few comments, tho:

> +(autoload 'mml-attach-file "mml"
> +  "Attach a file to the outgoing MIME message.
> +The file is not inserted or encoded until you send the message with
> +`\\[message-send-and-exit]' or `\\[message-send]'.
> +
> +FILE is the name of the file to attach.  TYPE is its
> +content-type, a string of the form \"type/subtype\".  DESCRIPTION
> +is a one-line description of the attachment.  The DISPOSITION
> +specifies how the attachment is intended to be displayed.  It can
> +be either \"inline\" (displayed automatically within the message
> +body) or \"attachment\" (separate from the body).")
> +
> +(autoload 'mm-default-file-encoding "mm-encode"
> +  "Return the default encoding for FILE.")

I must say I don't like such manually-managed autoloads.  I'd rather use
a declare-function plus an explicit require call.  My favorite would be
to use a new require-autoload construct which the byte-compiler would
replace by autoloads of the functions actually called (so the
byte-compiler would manage the list of autoloads rather than doing it by
hand).

> +\\[mail-attach-file] insert a text file into the message.
> +\\[mail-add-attachment] add to the message a file as a MIME attachment.

I think mail-attach-file needs to be renamed to mail-insert-file.


        Stefan

Re: mail-add-attachment

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Fri, 09 Sep 2011 10:32:19 -0400
> 
> > +(autoload 'mm-default-file-encoding "mm-encode"
> > +  "Return the default encoding for FILE.")
> 
> I must say I don't like such manually-managed autoloads.

Andreas suggested to add autoload cookies to the corresponding Gnus
files, but I see that both mml.el and mm-encode.el are entirely devoid
of such cookies, so I guess there are some Gnus coding standards
involved, which I don't want to break.

> I'd rather use a declare-function plus an explicit require call.

You mean, require in the function body, like below?

    (defun mail-add-attachment (file)
      "Add FILE as a MIME attachment to the end of the message."
      (interactive "fAttach file: ")
>>>>  (require 'mml)
>>>>  (require 'mm-encode)
      (mml-attach-file file
		       (or (mm-default-file-encoding file)
			   "application/octet-stream") nil)
      (setq mail-encode-mml t))

> My favorite would be to use a new require-autoload construct which
> the byte-compiler would replace by autoloads of the functions
> actually called (so the byte-compiler would manage the list of
> autoloads rather than doing it by hand).

I'll leave that project to you ;-)

> > +\\[mail-attach-file] insert a text file into the message.
> > +\\[mail-add-attachment] add to the message a file as a MIME attachment.
> 
> I think mail-attach-file needs to be renamed to mail-insert-file.

I thought about this, but isn't it too late to introduce incompatible
changes on the user level?  This command is there by that name since
1997.  I would at most go with a new name and an alias that is
deprecated.

Re: mail-add-attachment

[Lars Magne Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Andreas suggested to add autoload cookies to the corresponding Gnus
> files, but I see that both mml.el and mm-encode.el are entirely devoid
> of such cookies, so I guess there are some Gnus coding standards
> involved, which I don't want to break.

No, not at all.  If there are any coding standards involved on my part
(a dubious proposition :-), then it's "add any auto-load cookies that
would be useful (without littering the global Emacs name space with
functions that aren't useful)".

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

Re: mail-add-attachment

[Stefan Monnier]

> Andreas suggested to add autoload cookies to the corresponding Gnus
> files, but I see that both mml.el and mm-encode.el are entirely devoid
> of such cookies, so I guess there are some Gnus coding standards
> involved, which I don't want to break.

I don't have an opinion on whether autoload cookies should be placed in
those files.

>> I'd rather use a declare-function plus an explicit require call.
> You mean, require in the function body, like below?

>     (defun mail-add-attachment (file)
>       "Add FILE as a MIME attachment to the end of the message."
>       (interactive "fAttach file: ")
>>>>> (require 'mml)
>>>>> (require 'mm-encode)
>       (mml-attach-file file
> 		       (or (mm-default-file-encoding file)
> 			   "application/octet-stream") nil)
>       (setq mail-encode-mml t))

Yes.  It's not great, but it's better than manually managed autoload
cookies (which fail to include the (fn ARG1 AG2) calling
convention info, and are likely to get out of date).

>> My favorite would be to use a new require-autoload construct which
>> the byte-compiler would replace by autoloads of the functions
>> actually called (so the byte-compiler would manage the list of
>> autoloads rather than doing it by hand).
> I'll leave that project to you ;-)

Damn!

>> > +\\[mail-attach-file] insert a text file into the message.
>> > +\\[mail-add-attachment] add to the message a file as a MIME attachment.
>> I think mail-attach-file needs to be renamed to mail-insert-file.
> I thought about this, but isn't it too late to introduce incompatible
> changes on the user level?  This command is there by that name since
> 1997.  I would at most go with a new name and an alias that is
> deprecated.

Of course we'd keep an osolete alias.  But since we're making changes
anyway, I'd rather add that extra change to your patch.


        Stefan

Re: mail-add-attachment

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> > +\\[mail-attach-file] insert a text file into the message.
>>> > +\\[mail-add-attachment] add to the message a file as a MIME attachment.
>>> I think mail-attach-file needs to be renamed to mail-insert-file.
>> I thought about this, but isn't it too late to introduce incompatible
>> changes on the user level?  This command is there by that name since
>> 1997.  I would at most go with a new name and an alias that is
>> deprecated.
>
> Of course we'd keep an osolete alias.  But since we're making changes
> anyway, I'd rather add that extra change to your patch.

You want to rename the command that adds an attachment to
`mail-insert-file'?  Ok...  fine by me...

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

Re: mail-add-attachment

[Stefan Monnier]

>>>> > +\\[mail-attach-file] insert a text file into the message.
>>>> > +\\[mail-add-attachment] add to the message a file as a MIME attachment.
>>>> I think mail-attach-file needs to be renamed to mail-insert-file.
>>> I thought about this, but isn't it too late to introduce incompatible
>>> changes on the user level?  This command is there by that name since
>>> 1997.  I would at most go with a new name and an alias that is
>>> deprecated.
>> Of course we'd keep an osolete alias.  But since we're making changes
>> anyway, I'd rather add that extra change to your patch.
> You want to rename the command that adds an attachment to
> `mail-insert-file'?  Ok...  fine by me...

That's the thing: that command does not add an attachment, it's an old
non-MIME command that inserts the content of a text file into the email
text (with surrounding textual marks).  It was a poor man's
"attachment", but nowadays noone would be fooled into thinking that this
is an attachment.


        Stefan

Re: mail-add-attachment

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Fri, 09 Sep 2011 10:32:19 -0400
> 
> > Is it okay to install this command at this time?  I know we're in a
> > feature freeze, but the new command is actually a very thin wrapper
> > around code we already use elsewhere (gnus-dired.el and mml.el).  The
> > result will be that users of sendmail.el will be able to send MIME
> > attachments.  Here's the patch:
> 
> I guess it could be OK.

Thanks, committed as revision 105699 on the trunk.

> > +(autoload 'mm-default-file-encoding "mm-encode"
> > +  "Return the default encoding for FILE.")
> 
> I must say I don't like such manually-managed autoloads.

I added autoload cookies to the relevant Gnus files instead, per the
suggestion by Andreas.

> > +\\[mail-attach-file] insert a text file into the message.
> > +\\[mail-add-attachment] add to the message a file as a MIME attachment.
> 
> I think mail-attach-file needs to be renamed to mail-insert-file.

Done.