On Mon, 26 Dec 2016 14:10:05 +0000, Jun Wu wrote: > The goal is to move "pager" logic inside "runcommand" (both client and > server side) and remove the "getpager" API. And I'd like to discuss some > implementation details before sending patches. > > Problems: > > 1. Which channel to send "pagercmd" to the client? > 2. After sending "pagercmd" to client by chgui._runpager, how to deal with > "attachio"? > 3. How to arrange the signal handling / pager code client side? > > Proposed options: > > For 1, > > a) Reuse S channel. > The format to pass cwd, environ is reusable. > The format to read exitcode is not reusable. > An extra "command type" parameter is needed, which may be a minor BC. > (preferred if we don't care about such minor BC) > > b) Use a separate channel. > Will not BC old clients on the S channel format.
+1 for (a). I don't want to introduce new channel per s2c request. Regarding BC (IPC incompatibility), I'm thinking of adding a capability header to make sure the chg client is the version supported by the server, something like 'capabilities: chg-S-channel-2'. > For 2, > > a) Pass "attachio" or "chgcmdserver" to "_newchgui" so chgui can use it. > This is the most direct way. > (preferred) > > b) Move "attachio" related logic to chgui, and delegate chgcmdserver's > channels to ui. This seems unnecessarily complex and it does not seem > better than a. (a) seems okay as a start, but I don't think (b) is that complex. Since we're going to override the "runcommand" function, we will no longer need to replace server's cin, cout, and cerr channels. Which would mean the core logic of attachio() can be moved to ui (or a free function that takes ui.) > For 3, > > "pagerpid" is used in sighandler. Assuming we don't want new global > variables in all cases, I think the options are roughly: > > | pager | sighandler | new API | annotate history > -------------------------------------------------------------------- > now | chg.c | chg.c | - | no > a) | chg.c | chg.c | hgc_setrunpager | yes > | | | callback | > b) | hgclient.c | chg.c | hgc_getpagerpid | no > c) | util.c | util.c | - | no > d) | pager.c | sigutil.c | get/setpagerpid | yes > e) | procutil.c | procutil.c | - | yes > > I slightly prefer e) if we can find a good file name to contain both > pager and sighandler logic. (e) sounds nice since pager/sighandler stuffs are getting bigger. Thanks for the good summary about the pager refactoring. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
