[PATCH 1 of 6] exthelper: add a cautionary note about adding attributes to containers

[Matt Harbison]

On Thu, 27 Dec 2018 06:45:26 -0500, Yuya Nishihara <yuya at tcha.org> wrote:

> On Wed, 26 Dec 2018 16:33:33 -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at yahoo.com>
>> # Date 1545594763 18000
>> #      Sun Dec 23 14:52:43 2018 -0500
>> # Node ID 900b0af134505b6e5d227888212a17d8dd68342a
>> # Parent  0214962773921f15333f11d97212b307422aaaf5
>> exthelper: add a cautionary note about adding attributes to containers
>
> Queued, thanks.
>
>> diff --git a/mercurial/exthelper.py b/mercurial/exthelper.py
>> --- a/mercurial/exthelper.py
>> +++ b/mercurial/exthelper.py
>> @@ -255,6 +255,15 @@ class exthelper(object):
>>          This function takes two arguments, the container and the name  
>> of the
>>          function to wrap. The wrapping is performed during `uisetup`.
>>
>> +        Adding attributes to a container like this is discouraged,  
>> because the
>> +        container modification is visible even in repositories that do  
>> not
>> +        have the extension loaded.  Therefore, care must be taken that  
>> the
>> +        function doesn't make assumptions that the extension was  
>> loaded for the
>> +        current repository.  For `ui` and `repo` instances, a better  
>> option is
>> +        to subclass the instance in `uipopulate` and `reposetup`  
>> respectively.
>> +
>> +        https://www.mercurial-scm.org/wiki/WritingExtensions
>> +
>>          example::
>>
>>              @eh.addattr(context.changectx, 'babar')
>
> Can you update this example as well? Given most class instances belong to
> ui or repo, I think only valid use of addattr() is to add an attribute to
> module.

It looks like the existing uses in evolve are to modify the peer classes,  
and lfs is using it to modify the context.basefilectx class.  So I see  
what you mean about belonging to the repo instance, but am also surprised  
at the same time.

Can you think of a use case for adding an attribute to a module?  I  
suppose it doesn't matter too much, as the existing example is contrived  
too.