[PATCH 4 of 4] chgserver: move wrapchgui to runcommand
quark at fb.com
Sun Dec 18 13:24:45 EST 2016
Excerpts from Yuya Nishihara's message of 2016-12-18 17:30:31 +0900:
> On Fri, 16 Dec 2016 15:01:21 +0000, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu <quark at fb.com>
> > # Date 1481900282 0
> > # Fri Dec 16 14:58:02 2016 +0000
> > # Node ID 8fe60192f17f6ae99fa66c6bce1ec306772e31df
> > # Parent eb3017f14d56dfdc9870b06a684ef9bcf7a030e6
> > # Available At https://bitbucket.org/quark-zju/hg-draft
> > # hg pull https://bitbucket.org/quark-zju/hg-draft -r 8fe60192f17f
> > chgserver: move wrapchgui to runcommand
> > The wrapping logic changes ui.system, which should only affect runcommand.
> > This makes future refactoring a bit cleaner.
> > diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
> > --- a/mercurial/chgserver.py
> > +++ b/mercurial/chgserver.py
> > @@ -330,5 +330,4 @@ class chgcmdserver(commandserver.server)
> > def __init__(self, ui, repo, fin, fout, sock, hashstate, baseaddress):
> > self._csystem = channeledsystem(fin, fout, 'S')
> > - _wrapchgui(ui, self._csystem)
> > super(chgcmdserver, self).__init__(ui, repo, fin, fout)
> > self.clientsock = sock
> > @@ -507,4 +506,5 @@ class chgcmdserver(commandserver.server)
> > def runcommand(self):
> > + _wrapchgui(self.ui, self._csystem)
> > return super(chgcmdserver, self).runcommand()
> This change doesn't make sense to me. The first patch implies we need a global
> ui class having the knowledge about chg. This patch delays the introduction of
> the wrapped class, but still it lives in global space. (And you know,
> theoretically "runcommand" can be called more than once.)
> If we have to narrow the scope of csystem-ed ui without recreating it, we'll
> need a flag (or ui._csystem = None|object) to switch the behavior.
The direction is to eventually lose control on "ui" used in runcommand, and
let dispatch construct a "ui" object on its own. And we then use a special
"uisetup" which calls "wrapui" to modify the ui object created by dispatch.
Maybe this patch should be moved after adding "uisetup" in dispatch.request.
More information about the Mercurial-devel