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

Yuya Nishihara yuya at tcha.org
Wed Dec 21 08:20:10 EST 2016


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


More information about the Mercurial-devel mailing list