[PATCH 4 of 4] chgserver: move wrapchgui to runcommand

Jun Wu quark at fb.com
Tue Dec 20 09:03:04 EST 2016


Excerpts from Yuya Nishihara's message of 2016-12-20 21:47:23 +0900:
> On Mon, 19 Dec 2016 16:32:16 +0000, Jun Wu wrote:
> > Excerpts from Yuya Nishihara's message of 2016-12-19 23:46:26 +0900:
> > > On Sun, 18 Dec 2016 18:24:45 +0000, Jun Wu wrote:
> > > > The direction is to eventually lose control on "ui" used in runcommand, and
> > > > let dispatch construct a "ui" object on its own.
> > > 
> > > I got it.
> > > 
> > > > And we then use a special
> > > > "uisetup" which calls "wrapui" to modify the ui object created by dispatch.
> > > 
> > > Who will call this "uisetup"? dispatch or chgserver.runcommand?
> > 
> > The plan is to add a "uisetup" argument to dispatch.request:
> > 
> >   diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> >   --- a/mercurial/dispatch.py
> >   +++ b/mercurial/dispatch.py
> >   @@ -49,5 +49,5 @@ from . import (
> >    class request(object):
> >        def __init__(self, args, ui=None, repo=None, fin=None, fout=None,
> >   -                 ferr=None):
> >   +                 ferr=None, uisetup=None):
> >            self.args = args
> >            self.ui = ui
> >   @@ -59,4 +59,7 @@ class request(object):
> >            self.ferr = ferr
> >    
> >   +        # an extra uisetup unrelated to extensions, used by chg
> >   +        self.uisetup = uisetup
> >   +
> >    def run():
> >        "run the command in sys.argv"
> >   @@ -660,4 +663,9 @@ def _dispatch(req):
> >        extensions.loadall(lui)
> >        exts = [ext for ext in extensions.extensions() if ext[0] not in _loaded]
> >   +
> >   +    # An extra uisetup of the request, currently used by chg
> >   +    if req.uisetup is not None:
> >   +        req.uisetup(lui)
> >   +
> >        # Propagate any changes to lui.__class__ by extensions
> >        ui.__class__ = lui.__class__
> 
> Ah, that seems okay.
> 
> BTW, is there any reason we have to delay the uisetup() call? I think we can
> just set req.ui in place of req.uisetup:
> 
>   class chgui(uimod.ui):
>       ...
>   req = dispatch.request(ui=chgui.load())

It's useful if runcommand needs to wrap on top of the side effects of other
extensions (ex. pager). In my WIP patch, chgcmdserver.uisetup looks like:

    def _uisetup(self, ui):
        _wrapui(ui, self._csystem)
        try:
            pager = extensions.find('pager')
        except KeyError:
            pass
        else:
            if util.safehasattr(pager, '_runpager'):
                extensions.wrapfunction(pager, '_runpager', self._runpager)

    def _runpager(self, orig, ui, pagercmd):
        self._csystem.write(pagercmd, type='pager')
        while True:
            cmd = self.client.readline()[:-1]
            _log('pager subcommand: %s' % cmd)
            if cmd == 'attachio':
                self.attachio(ui)
            elif cmd == '':
                break
            else:
                raise error.Abort(_('unexpected command %s') % cmd)

_runpager is coupled with chgcmdserver.

So uisetup is still necessary to wrap _runpager. The _wrapui change is
unnecessary if we use your proposal. In that case, both Patch 2 and 4 can be
dropped and Patch 1 and 3 are useful.

> > > I'm slightly afraid of modifying ui class in the middle of the server session
> > > since the ui might be used after runcommand(). That could lead  to a subtle bug.
> > 
> > I agree. So patch 4 should be deferred until the uisetup work is done. Since
> > that depends on a lot of other things. Patch 4 should be dropped now.


More information about the Mercurial-devel mailing list