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. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel