[PATCH 5 of 5] commands: always register special command properties in registrar (API)

[Yuya Nishihara]

On Sat, 2 Jan 2016 18:11:07 -0800, Gregory Szorc wrote:
> On Sat, Jan 2, 2016 at 5:51 PM, Yuya Nishihara <yuya at tcha.org> wrote:
> > On Sat, 02 Jan 2016 11:30:54 -0800, Gregory Szorc wrote:
> > > # HG changeset patch
> > > # User Gregory Szorc <gregory.szorc at gmail.com>
> > > # Date 1451761491 28800
> > > #      Sat Jan 02 11:04:51 2016 -0800
> > > # Node ID 3ed88f67769061aec1a6de827f739c3f65463952
> > > # Parent  f77ea53bf73191d7fa373dddb0fbd03f3cb61b12
> > > commands: always register special command properties in registrar (API)
> > >
> > > The previous patch to start the refactor of this mechanism
> > > intentionally did not modify dispatch.py. This patch starts the process
> > > of teaching dispatch.py to be registrar aware.
> > >
> > > We still end up setting values in commands.py. But, we no longer do this
> > > from @command and commands.py no longer needs to know about registrar.
> >
> > Uh, I have experimental patches that store command properties in function
> > object.
> >
> > https://bitbucket.org/yuja/hg-work/commits/7012d4af63af
> >
> > Pros:
> >
> >  - no circular import
> >  - no change to global variables at _checkshellalias()
> >
> > Cons:
> >
> >  - these attributes must be copied to a wrapper function
> >
> 
> Storing these properties in the function object is a much better approach.
> 
> The impetus behind these patches was getting cmdutil and commands converted
> to absolute_import so the Python 3 porting could be unblocked. I concede
> that porting commands.{norepo,optionalrepo,inferrepo} is sub-optimal.
> 
> The Python 3 work is low priority. If you want to convert these to function
> object attributes, I'm fine with dropping this series and picking up
> whenever your changes land.

Thanks, I'll finish my series and patchbomb it soon.

> > > This is marked as API incompatible because extensions examining
> > > commands.{norepo,optionalrepo,inferrepo} will no longer work since this
> > > variable is no longer populated at extension load time. This is
> > > demonstrated by the simple change to mq.py.
> >
> > If we can drop commands.{norepo,optionalrepo,inferrepo}, API compatibility
> > issues will be more discoverable.
> >
> 
> I'd like to drop them too. But I think it may have significant implications
> on legacy extensions. Perhaps this is a mpm decision?

CC-ing mpm. :-)