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

Yuya Nishihara yuya at tcha.org
Mon Dec 19 09:46:26 EST 2016


On Sun, 18 Dec 2016 18:24:45 +0000, Jun Wu wrote:
> 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.

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?

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.

> Maybe this patch should be moved after adding "uisetup" in dispatch.request.

If it is a temporary code, I don't care much about the cleanliness.


More information about the Mercurial-devel mailing list