On Tue, 20 Dec 2016 16:46:38 +0000, 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 +0000, 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
