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.
+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
