[MERGE] Mail clients listed in a registry

[Neil Martinsen-Burrell]

On 2008-08-19, at 11:29 AM, John Arbash Meinel wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Neil Martinsen-Burrell wrote:
>> Here is a patch to make mail clients extensible by plugins by listing
>> them in a registry. While Bazaar may reasonably try to support all
>> popular mail clients, there will always be niche programs and  
>> platforms
>> that we can't support.  There will also be times where some vaporware
>> exists to support a certain client, but it hasn't landed yet.   
>> (This is
>> currently the case with Mail.app.)
>>
>> The patch makes it possible to add a mail client from a plugin as the
>> attached plugin demonstrates.  (This email sent using Mail.app and  
>> that
>> plugin.)  Test coverage is still adequate as we currently test the
>> getting of mail_clients from Config objects for every built-in  
>> client.
>>
>> -Neil
>>
>
> Hmm.. weird.
> It seems your __init__.py script was shown inline, but your bzr  
> patch was not.
> I started reviewing it, thinking it was the patch. It might have  
> been a
> little better to do 2 separate emails. (__init__.py doesn't really  
> conform to
> some of our coding standards, but it doesn't really matter as it is  
> *your*
> plugin.)
>
>
> +from bzrlib.mail_client import mail_client_registry
> """)
>
> ^- I really prefer it if lazy_import is only done on module objects.  
> So
> instead do:
>
> from bzrlib import mail_client
>
> ...
>  mail_client.mail_client_registry.get(selected_client)
>
> (If there is already a "from bzrlib import" then mail_client should be
> inserted into that list.)
>

Done, with appropriate 80 column contortions

> +mail_client_registry.register_lazy(
> +                'editor',
> +                'bzrlib.mail_client',
> +                'Editor')
>
> ^- There is no reason to use "register_lazy" as you have declared  
> the class
> *right there*. 'register_lazy' is for when the declared class may  
> not need to
> be imported at all.
>
> For example, if you defined the mail_client_registry in config.py  
> and then
> used 'register_lazy' on everything.
>
> In this particular case, I'm happier seeing 'mail_client_registry' in
> mail_client.py and having the mail_client.py module not get loaded  
> until you
> need it.
>
> So these should generally become:
>
> mail_client_registry.register('editor', Editor)

Done.  Much cleaner.  I had just cribbed from version_info_formats (my  
other experience with a Bazaar registry) but there it is indeed  
referencing other modules which define the formats.

> Eventually it might be nice to add help strings, as then we can auto- 
> generate
> documentation for them. But that is certainly outside the scope of  
> your change.

Done.  The docstring is used as the help string for each of the built- 
in clients.  (I didn't think I would ever want a class decorator, but  
one would be awesome for this; register the above class under a  
certain key using its docstring as the help.)

-Neil

-------------- next part --------------
A non-text attachment was scrubbed...
Name: email-client-registry-3643.patch
Type: application/octet-stream
Size: 10817 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080819/22be836d/attachment.obj