[MERGE] Replace mail-mode call with compose-mail from GNU Emacs.

Ian Clatworthy ian.clatworthy at internode.on.net
Wed Apr 2 02:26:43 BST 2008


Xavier Maillard wrote:
> This patch is IMO prefered to revno 3323. It will apply on the
> tip of bzr.dev safely (no need to revert revno 3323 as requested
> earlier today).

bb: tweak

Thanks. If you decide to improve the error message as mentioned below,
then I think it's worth resubmitting this. Otherwise, it's good enough
for me given the tweaks below. (I should explicitly mention though that
I'm not using Emacs so I'm unable to test this end to end.)

> --- NEWS	2008-04-01 04:19:06 +0000
> +++ NEWS	2008-04-01 17:51:16 +0000
> @@ -1,3 +1,4 @@
> +

This extra line at the top of the file shouldn't be there.

> +      ``send` will pop the bundle in a mail buffer according to the

Need an extra ` after send.

> +    The good news is that this implementation will work with all mail
> +    agent registered against ...

agents, not agent here.

> +        _defun = """(defun bzr-add-mime-att (file)
> +  \"Attach FILe to a mail buffer as a MIME attachment.\"
> +  (let ((agent mail-user-agent))
> +    (mail-text)
> +    (newline)
> +    (if (and file (file-exists-p file))
> +	(cond
> +	 ((eq agent \'sendmail-user-agent)
> +	  (etach-attach file))
> +	 ((or (eq agent \'message-user-agent)(eq agent \'gnus-user-agent))
> +	  (mml-attach-file file \"text/x-patch\" \"BZR merge\" \"attachement\"))
> +	 (t
> +	  (message \"Unhandled MUA\")))
> +      (error \"File does not exist.\"))))
> +"""

If you use a raw string here, you don't need to escape ' and ". A raw
string can be specified using r""" ... """. Also:

* The spelling of attachment is incorrect on the mml-attach-file line.

* Is it worth mentioning which file does not exist in the error
  message? Will the user have enough context if they receive that
  message?

> +            lmmform ="(load \"%s\")" % self._prepare_send_function()
> +            mmform  = "(bzr-add-mime-att \"%s\")" % self._encode_path(attach_path, 'attachment')

The first line needs a space after the =. The second line is over 80
chars. BTW, you can also cut down on the escaping needed on these lines
and a few above them by using single quotes for the outer delimiters, e.g.

  mmform  = '(bzr-add-mime-att "%s")' % ...

> +	# We won't be able to know the temporary file name at this stage
> +	# so we can't raise an assertion with assertEqual
> +        cmdline = eclient._get_compose_commandline(None, None, 'file%')

These comments should be indented the same amount as the code they apply
to. It looks like you have tabs before the comments. This renders
incorrectly for those of us with our tab indent set to 4. Please use
spaces everywhere.

Ian C.



More information about the bazaar mailing list