Send actually *sends* the merge directive

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

A prefix: Your preview patch could not be verified.  That means my
machine generated a preview patch using the same parameters as yours
did, and the resulting patch was different from yours.  My machine used
PatienceSequenceMatcher.  Was yours using the C version?  If the C
sequence matcher produces different output, that would explain it.

It would also mean we can't use the C version for generating or
verifying preview patches.

Lukáš Lalinský wrote:
> I've found a better library here:
> 
> http://www.johnnypops.demon.co.uk/python/
>
> The main advantage is that it doesn't use COM, only ctypes, which is
> already bzr's dependency. I've tested it with Outlook and Outlook
> Express and it seems to work fine.

I can't see any licensing information about this library, or its
progenitor: http://www.kirbyfooty.com/simplemapi.py

If it's not under a GPL-compatible license, we can't use it.

> === modified file 'bzrlib/config.py'
> --- bzrlib/config.py	2007-08-10 17:34:14 +0000
> +++ bzrlib/config.py	2007-08-10 19:06:11 +0000
> @@ -154,6 +154,7 @@
>                  'editor': mail_client.Editor,
>                  'thunderbird': mail_client.Thunderbird,
>                  'evolution': mail_client.Evolution,
> +                'windows': mail_client.Windows,

^^^ Not sure whether 'mapi' would be better than 'windows'.  This is a
technical-user's option, since the default will Just Work.

But why is it mail_client.Windows here and MAPIClient in mail_client?  I
suspect 'windows' doesn't work at all.  You should really add a test for
it to test_config.test_get_mail_client

> +lazy_import(globals(), '''
>  import errno
> -import os
>  import subprocess
>  import tempfile
>  
> @@ -25,6 +29,8 @@
>      msgeditor,
>      urlutils,
>      )
> +from bzrlib.util import simplemapi
> +''')

^^^ I don't think we should use lazy_import to prevent something from
being loaded.  Conditional imports should be done explicitly, e.g. "if
osutils.supports_mapi(): import simplemapi"

But in this case, it would be sensible to import it in
MAPIClient._compose.  It won't hurt performance significantly.

> -class Evolution(MailClient):
> -    """Evolution mail client."""
> +class ExternalMailClient(MailClient):
> +    """An external mail client."""
>  
> -    _client_commands = ['evolution']
> +    _client_commands = []

^^^ Extracting ExternalMailClient from Evolution is fine, but I don't
think it needs a _client_commands list, especially since it's empty.

> @@ -80,10 +86,14 @@
>              os.write(fd, attachment)
>          finally:
>              os.close(fd)
> +        self._compose(prompt, to, subject, pathname, mime_subtype, extension)
> +
> +    def _compose(self, prompt, to, subject, attach_path, mime_subtype,
> +                extension):
>          for name in self._client_commands:
>              cmdline = [name]
> -            cmdline.extend(self._get_compose_commandline(to, subject,
> -                                                         pathname))
> +            cmdline.extend(self._get_compose_commandline(to, subject, 
> +                                                         attach_path))
>              try:
>                  subprocess.call(cmdline)
>              except OSError, e:
> @@ -105,7 +115,23 @@
>          return ['mailto:%s?%s' % (to or '', '&'.join(options_list))]
>  
>  
> -class Thunderbird(Evolution):
> +class Evolution(ExternalMailClient):
> +    """Evolution mail client."""
> +
> +    _client_commands = ['evolution']
> +
> +    def _get_compose_commandline(self, to, subject, attach_path):
> +        message_options = {}
> +        if subject is not None:
> +            message_options['subject'] = subject
> +        if attach_path is not None:
> +            message_options['attach'] = attach_path
> +        options_list = ['%s=%s' % (k, urlutils.escape(v)) for (k, v) in
> +                        message_options.iteritems()]
> +        return ['mailto:%s?%s' % (to or '', '&'.join(options_list))]

^^^ This appears to duplicate ExternalMailClient._get_compose_commandline


> +class MAPIClient(ExternalMailClient):
> +    """Default Windows mail client launched using MAPI."""
> +
> +    def _compose(self, prompt, to, subject, attach_path, mime_subtype,
> +                 extension):
> +        simplemapi.SendMail(to or '', subject or '', '', attach_path)

^^^ Is it possible for simplemapi.SendMail to fail?

>  class DefaultMail(MailClient):
> -    """Default mail handling.  Tries XDGEmail, falls back to Editor"""
> +    """Default mail handling.  Tries XDGEmail (or MAPIClient on Windows),
> +    falls back to Editor"""
> +
> +    def _mail_client(self):
> +        if sys.platform == 'win32':
> +            return MAPIClient(self.config)

^^^ We generally abstract out platform checks and turn them into feature
checks, like osutils.supports_executable().

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGvMW80F+nu1YWqI0RAg+CAJoCIsCXEb4j5qxbqQClYjnpmHMd8ACfT3iP
0wxmgSijrXgczOUHBUDRlTU=
=t8a9
-----END PGP SIGNATURE-----