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

[Matt Mackall]

On Mon, 2016-01-04 at 00:17 +0900, Yuya Nishihara wrote:
> 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. :-)

I love to break extensions, but perhaps we shouldn't do it so wholesale so close
to the freeze.

-- 
Mathematics is the supreme nostalgia of our time.