[PATCH 4 of 4] chgserver: move wrapchgui to runcommand
# HG changeset patch # User Jun Wu # Date 1481900282 0 # Fri Dec 16 14:58:02 2016 + # 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() ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
On Fri, 16 Dec 2016 15:01:21 +, Jun Wu wrote: > # HG changeset patch > # User Jun Wu > # Date 1481900282 0 > # Fri Dec 16 14:58:02 2016 + > # 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. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
Excerpts from Yuya Nishihara's message of 2016-12-18 17:30:31 +0900: > On Fri, 16 Dec 2016 15:01:21 +, Jun Wu wrote: > > # HG changeset patch > > # User Jun Wu > > # Date 1481900282 0 > > # Fri Dec 16 14:58:02 2016 + > > # 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. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
On Sun, 18 Dec 2016 18:24:45 +, 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 +, Jun Wu wrote: > > > # HG changeset patch > > > # User Jun Wu > > > # Date 1481900282 0 > > > # Fri Dec 16 14:58:02 2016 + > > > # 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. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
Excerpts from Yuya Nishihara's message of 2016-12-19 23:46:26 +0900: > On Sun, 18 Dec 2016 18:24:45 +, 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__ > 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. > > 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. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
On Mon, 19 Dec 2016 16:32:16 +, 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 +, 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()) > > 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
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
Excerpts from Yuya Nishihara's message of 2016-12-20 21:47:23 +0900:
> On Mon, 19 Dec 2016 16:32:16 +, 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 +, 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
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
On Tue, 20 Dec 2016 14:03:04 +, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2016-12-20 21:47:23 +0900:
> > 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.
Could it be implemented without req.uisetup() if we had ui.pager() function?
https://www.mercurial-scm.org/wiki/PagerInCorePlan
I believe we'll need to refactor the pager handling to fix a couple of pager
issues (e.g. issue5377.) So I'm not enthusiastic about this _runpager() change.
> 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.
Yep, that's why I haven't pushed these patches yet.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
Excerpts from Yuya Nishihara's message of 2016-12-20 23:29:17 +0900:
> On Tue, 20 Dec 2016 14:03:04 +, Jun Wu wrote:
> > Excerpts from Yuya Nishihara's message of 2016-12-20 21:47:23 +0900:
> > > 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.
>
> Could it be implemented without req.uisetup() if we had ui.pager() function?
>
> https://www.mercurial-scm.org/wiki/PagerInCorePlan
>
> I believe we'll need to refactor the pager handling to fix a couple of pager
> issues (e.g. issue5377.) So I'm not enthusiastic about this _runpager()
> change.
I was aware of the pager refactoring. If we can figure out the final APIs,
and decouple the complex pager refactoring so the part needed by chg is
small, I can do that.
The pager API has 2 levels:
- high-level (pagecmd): decide the command of the pager, call low-level
- low-level (_runpager): accept a command and run it unconditionally
I think ui.pager() should be high-level, and chg only wants to replace the
low-level one.
Therefore a possible API is:
- ui.pager() as the new high-level API which parses config and calls
low-level method.
- ui._runpager(pagercmd) as the low-level API which will be implemented
differently by chg.
A possible approach is:
1. Move pager._runpager to uimod.ui._runpager
2. Override ui._runpager in chgui
3. Move part of pagecmd to uimod.ui.pager (or startpager if we plan to
have an endpager in the future)
4. Revisit when to call ui.pager (complex)
1 and 2 are easy and related to chg. 3 does not block chg refactoring and is
simple enough so I could help by the way. 4 is a complex core part of the
pager plan but I'd like to avoid as it has nothing to do with chg.
+durin42 because of pager.
> > 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.
>
> Yep, that's why I haven't pushed these patches yet.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
On Tue, 20 Dec 2016 16:46:38 +, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2016-12-20 23:29:17 +0900:
> > On Tue, 20 Dec 2016 14:03:04 +, Jun Wu wrote:
> > > Excerpts from Yuya Nishihara's message of 2016-12-20 21:47:23 +0900:
> > > > 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.
> >
> > Could it be implemented without req.uisetup() if we had ui.pager() function?
> >
> > https://www.mercurial-scm.org/wiki/PagerInCorePlan
> >
> > I believe we'll need to refactor the pager handling to fix a couple of pager
> > issues (e.g. issue5377.) So I'm not enthusiastic about this _runpager()
> > change.
>
> I was aware of the pager refactoring. If we can figure out the final APIs,
> and decouple the complex pager refactoring so the part needed by chg is
> small, I can do that.
>
> The pager API has 2 levels:
>
> - high-level (pagecmd): decide the command of the pager, call low-level
> - low-level (_runpager): accept a command and run it unconditionally
>
> I think ui.pager() should be high-level, and chg only wants to replace the
> low-level one.
>
> Therefore a possible API is:
>
> - ui.pager() as the new high-level API which parses config and calls
> low-level method.
> - ui._runpager(pagercmd) as the low-level API which will be implemented
> differently by chg.
>
> A possible approach is:
>
> 1. Move pager._runpager to uimod.ui._runpager
> 2. Override ui._runpager in chgui
> 3. Move part of pagecmd to uimod.ui.pager (or startpager if we plan to
> have an endpager in the future)
> 4. Revisit when to call ui.pager (complex)
>
> 1 and 2 are easy and related to chg. 3 does not block chg refactoring and is
> simple enough so I could help by the way. 4 is a complex core part of the
> pager plan but I'd like to avoid as it has nothing to do with chg.
Sounds reasonable to me. I had something similar in mind, which would only
implement the low-level API:
1. add ui._runpager() -> _runpager()
2. s/_runpager()/ui._runpager()/
3. override ui._runpager() by chgui
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
On Tue, 20 Dec 2016 14:03:04 +, Jun Wu wrote: > 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've queued 1 and 3, thanks. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
Excerpts from Yuya Nishihara's message of 2016-12-21 23:15:28 +0900: > On Tue, 20 Dec 2016 14:03:04 +, Jun Wu wrote: > > 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've queued 1 and 3, thanks. Actually, patch 1 is unnecessary if we go with the "ui._runpager" approach. Maybe someone can drop it without adding too many markers. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
On Wed, 21 Dec 2016 15:39:05 +, Jun Wu wrote: > Excerpts from Yuya Nishihara's message of 2016-12-21 23:15:28 +0900: > > On Tue, 20 Dec 2016 14:03:04 +, Jun Wu wrote: > > > 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've queued 1 and 3, thanks. > > Actually, patch 1 is unnecessary if we go with the "ui._runpager" approach. > Maybe someone can drop it without adding too many markers. That's probably me. I'll drop the patch: dfb19aed409e "chgserver: store csystem separately." ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand
> On Dec 20, 2016, at 11:46, Jun Wu wrote: > > The pager API has 2 levels: > > - high-level (pagecmd): decide the command of the pager, call low-level > - low-level (_runpager): accept a command and run it unconditionally > > I think ui.pager() should be high-level, and chg only wants to replace the > low-level one. > > Therefore a possible API is: > > - ui.pager() as the new high-level API which parses config and calls >low-level method. > - ui._runpager(pagercmd) as the low-level API which will be implemented >differently by chg. > > A possible approach is: > > 1. Move pager._runpager to uimod.ui._runpager > 2. Override ui._runpager in chgui > 3. Move part of pagecmd to uimod.ui.pager (or startpager if we plan to > have an endpager in the future) > 4. Revisit when to call ui.pager (complex) > > 1 and 2 are easy and related to chg. 3 does not block chg refactoring and is > simple enough so I could help by the way. 4 is a complex core part of the > pager plan but I'd like to avoid as it has nothing to do with chg. This matches my understanding of where we probably have to go for the pager work. I'm not sure if we should plan to make the pager exit-able, but that depends on the architecture we use for paging, which I haven't given much thought lately. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
