Patchwork [7,of,8] chg: remove getpager support

login
register
mail settings
Submitter Jun Wu
Date Jan. 9, 2017, 11:12 p.m.
Message ID <24c7b11cecd12762d823.1484003547@x1c>
Download mbox | patch
Permalink /patch/18159/
State Accepted
Headers show

Comments

Jun Wu - Jan. 9, 2017, 11:12 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1484002779 -28800
#      Tue Jan 10 06:59:39 2017 +0800
# Node ID 24c7b11cecd12762d8238ec96c748131877054be
# Parent  e44b1d6deadb67908b43909ad920572e3efa38e6
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 24c7b11cecd1
chg: remove getpager support

We have enough bits to switch to the new chg pager code path in runcommand.
So just remove the legacy getpager support.

This is a red-only patch, and will break chg's pager support temporarily.
Yuya Nishihara - Jan. 10, 2017, 2:45 p.m.
On Tue, 10 Jan 2017 07:12:27 +0800, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1484002779 -28800
> #      Tue Jan 10 06:59:39 2017 +0800
> # Node ID 24c7b11cecd12762d8238ec96c748131877054be
> # Parent  e44b1d6deadb67908b43909ad920572e3efa38e6
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 24c7b11cecd1
> chg: remove getpager support
> 
> We have enough bits to switch to the new chg pager code path in runcommand.
> So just remove the legacy getpager support.
> 
> This is a red-only patch, and will break chg's pager support temporarily.

I love red patches. Queued the series, thanks!
Pierre-Yves David - Jan. 10, 2017, 5:22 p.m.
On 01/10/2017 03:45 PM, Yuya Nishihara wrote:
> On Tue, 10 Jan 2017 07:12:27 +0800, Jun Wu wrote:
>> # HG changeset patch
>> # User Jun Wu <quark@fb.com>
>> # Date 1484002779 -28800
>> #      Tue Jan 10 06:59:39 2017 +0800
>> # Node ID 24c7b11cecd12762d8238ec96c748131877054be
>> # Parent  e44b1d6deadb67908b43909ad920572e3efa38e6
>> # Available At https://bitbucket.org/quark-zju/hg-draft
>> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 24c7b11cecd1
>> chg: remove getpager support
>>
>> We have enough bits to switch to the new chg pager code path in runcommand.
>> So just remove the legacy getpager support.
>>
>> This is a red-only patch, and will break chg's pager support temporarily.
>
> I love red patches. Queued the series, thanks!

Hooray for removing much code!
I'm curious about the breakage however. It is not too important because 
chg is experimental but is that breakage fixed by the next changeset? 
could have reordered or folded this patch to avoid being temporarily broken?

Cheers,
Jun Wu - Jan. 11, 2017, 3:40 a.m.
Excerpts from Pierre-Yves David's message of 2017-01-10 18:22:39 +0100:
> 
> On 01/10/2017 03:45 PM, Yuya Nishihara wrote:
> > On Tue, 10 Jan 2017 07:12:27 +0800, Jun Wu wrote:
> >> # HG changeset patch
> >> # User Jun Wu <quark@fb.com>
> >> # Date 1484002779 -28800
> >> #      Tue Jan 10 06:59:39 2017 +0800
> >> # Node ID 24c7b11cecd12762d8238ec96c748131877054be
> >> # Parent  e44b1d6deadb67908b43909ad920572e3efa38e6
> >> # Available At https://bitbucket.org/quark-zju/hg-draft 
> >> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 24c7b11cecd1
> >> chg: remove getpager support
> >>
> >> We have enough bits to switch to the new chg pager code path in runcommand.
> >> So just remove the legacy getpager support.
> >>
> >> This is a red-only patch, and will break chg's pager support temporarily.
> >
> > I love red patches. Queued the series, thanks!
> 
> Hooray for removing much code!
> I'm curious about the breakage however. It is not too important because 
> chg is experimental but is that breakage fixed by the next changeset? 
> could have reordered or folded this patch to avoid being temporarily broken?

If Patch 7 and Patch 8 were folded, there would be no breakage.

They are separated to comply with the small commit workflow.

> 
> Cheers,
>
Pierre-Yves David - Jan. 11, 2017, 7:51 a.m.
On 01/11/2017 04:40 AM, Jun Wu wrote:
> Excerpts from Pierre-Yves David's message of 2017-01-10 18:22:39 +0100:
>>
>> On 01/10/2017 03:45 PM, Yuya Nishihara wrote:
>>> On Tue, 10 Jan 2017 07:12:27 +0800, Jun Wu wrote:
>>>> # HG changeset patch
>>>> # User Jun Wu <quark@fb.com>
>>>> # Date 1484002779 -28800
>>>> #      Tue Jan 10 06:59:39 2017 +0800
>>>> # Node ID 24c7b11cecd12762d8238ec96c748131877054be
>>>> # Parent  e44b1d6deadb67908b43909ad920572e3efa38e6
>>>> # Available At https://bitbucket.org/quark-zju/hg-draft
>>>> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 24c7b11cecd1
>>>> chg: remove getpager support
>>>>
>>>> We have enough bits to switch to the new chg pager code path in runcommand.
>>>> So just remove the legacy getpager support.
>>>>
>>>> This is a red-only patch, and will break chg's pager support temporarily.
>>>
>>> I love red patches. Queued the series, thanks!
>>
>> Hooray for removing much code!
>> I'm curious about the breakage however. It is not too important because
>> chg is experimental but is that breakage fixed by the next changeset?
>> could have reordered or folded this patch to avoid being temporarily broken?
>
> If Patch 7 and Patch 8 were folded, there would be no breakage.
>
> They are separated to comply with the small commit workflow.

For the record, I put the "no breakage between commit" principle before 
the "small commit submission" one (and we can usually adapt the slicing 
of commit to keep both).

(But, that's not too important in this case so I've accepted these patch 
yesterday anyway)

Patch

diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
--- a/contrib/chg/chg.c
+++ b/contrib/chg/chg.c
@@ -430,8 +430,4 @@  int main(int argc, const char *argv[], c
 
 	setupsignalhandler(hgc_peerpid(hgc), hgc_peerpgid(hgc));
-	const char *pagercmd = hgc_getpager(hgc, argv + 1, argc - 1);
-	pid_t pagerpid = setuppager(pagercmd);
-	if (pagerpid)
-		hgc_attachio(hgc);  /* reattach to pager */
 	int exitcode = hgc_runcommand(hgc, argv + 1, argc - 1);
 	restoresignalhandler();
diff --git a/contrib/chg/hgclient.c b/contrib/chg/hgclient.c
--- a/contrib/chg/hgclient.c
+++ b/contrib/chg/hgclient.c
@@ -33,5 +33,4 @@  enum {
 	CAP_ATTACHIO = 0x0100,
 	CAP_CHDIR = 0x0200,
-	CAP_GETPAGER = 0x0400,
 	CAP_SETENV = 0x0800,
 	CAP_SETUMASK = 0x1000,
@@ -49,5 +48,4 @@  static const cappair_t captable[] = {
 	{"attachio", CAP_ATTACHIO},
 	{"chdir", CAP_CHDIR},
-	{"getpager", CAP_GETPAGER},
 	{"setenv", CAP_SETENV},
 	{"setumask", CAP_SETUMASK},
@@ -594,29 +592,4 @@  void hgc_attachio(hgclient_t *hgc)
 
 /*!
- * Get pager command for the given Mercurial command args
- *
- * If no pager enabled, returns NULL. The return value becomes invalid
- * once you run another request to hgc.
- */
-const char *hgc_getpager(hgclient_t *hgc, const char *const args[],
-			 size_t argsize)
-{
-	assert(hgc);
-
-	if (!(hgc->capflags & CAP_GETPAGER))
-		return NULL;
-
-	packcmdargs(&hgc->ctx, args, argsize);
-	writeblockrequest(hgc, "getpager");
-	handleresponse(hgc);
-
-	if (hgc->ctx.datasize < 1 || hgc->ctx.data[0] == '\0')
-		return NULL;
-	enlargecontext(&hgc->ctx, hgc->ctx.datasize + 1);
-	hgc->ctx.data[hgc->ctx.datasize] = '\0';
-	return hgc->ctx.data;
-}
-
-/*!
  * Update server's environment variables
  *
diff --git a/contrib/chg/hgclient.h b/contrib/chg/hgclient.h
--- a/contrib/chg/hgclient.h
+++ b/contrib/chg/hgclient.h
@@ -26,6 +26,4 @@  const char **hgc_validate(hgclient_t *hg
 int hgc_runcommand(hgclient_t *hgc, const char *const args[], size_t argsize);
 void hgc_attachio(hgclient_t *hgc);
-const char *hgc_getpager(hgclient_t *hgc, const char *const args[],
-			 size_t argsize);
 void hgc_setenv(hgclient_t *hgc, const char *const envp[]);
 
diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
--- a/mercurial/chgserver.py
+++ b/mercurial/chgserver.py
@@ -17,7 +17,4 @@ 
     change current directory
 
-'getpager' command
-    checks if pager is enabled and which pager should be executed
-
 'setenv' command
     replace os.environ completely
@@ -46,5 +43,4 @@  import inspect
 import os
 import re
-import signal
 import struct
 import time
@@ -53,5 +49,4 @@  from .i18n import _
 
 from . import (
-    cmdutil,
     commandserver,
     encoding,
@@ -173,43 +168,4 @@  class hashstate(object):
         return hashstate(confighash, mtimehash, mtimepaths)
 
-# copied from hgext/pager.py:uisetup()
-def _setuppagercmd(ui, options, cmd):
-    from . import commands  # avoid cycle
-
-    if not ui.formatted():
-        return
-
-    p = ui.config("pager", "pager", encoding.environ.get("PAGER"))
-    usepager = False
-    always = util.parsebool(options['pager'])
-    auto = options['pager'] == 'auto'
-
-    if not p:
-        pass
-    elif always:
-        usepager = True
-    elif not auto:
-        usepager = False
-    else:
-        attended = ['annotate', 'cat', 'diff', 'export', 'glog', 'log', 'qdiff']
-        attend = ui.configlist('pager', 'attend', attended)
-        ignore = ui.configlist('pager', 'ignore')
-        cmds, _ = cmdutil.findcmd(cmd, commands.table)
-
-        for cmd in cmds:
-            var = 'attend-%s' % cmd
-            if ui.config('pager', var):
-                usepager = ui.configbool('pager', var)
-                break
-            if (cmd in attend or
-                (cmd not in ignore and not attend)):
-                usepager = True
-                break
-
-    if usepager:
-        ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
-        ui.setconfig('ui', 'interactive', False, 'pager')
-        return p
-
 def _newchgui(srcui, csystem, attachio):
     class chgui(srcui.__class__):
@@ -485,35 +441,4 @@  class chgcmdserver(commandserver.server)
         os.umask(mask)
 
-    def getpager(self):
-        """Read cmdargs and write pager command to r-channel if enabled
-
-        If pager isn't enabled, this writes '\0' because channeledoutput
-        does not allow to write empty data.
-        """
-        from . import dispatch  # avoid cycle
-
-        args = self._readlist()
-        try:
-            cmd, _func, args, options, _cmdoptions = dispatch._parse(self.ui,
-                                                                     args)
-        except (error.Abort, error.AmbiguousCommand, error.CommandError,
-                error.UnknownCommand):
-            cmd = None
-            options = {}
-        if not cmd or 'pager' not in options:
-            self.cresult.write('\0')
-            return
-
-        pagercmd = _setuppagercmd(self.ui, options, cmd)
-        if pagercmd:
-            # Python's SIGPIPE is SIG_IGN by default. change to SIG_DFL so
-            # we can exit if the pipe to the pager is closed
-            if util.safehasattr(signal, 'SIGPIPE') and \
-                    signal.getsignal(signal.SIGPIPE) == signal.SIG_IGN:
-                signal.signal(signal.SIGPIPE, signal.SIG_DFL)
-            self.cresult.write(pagercmd)
-        else:
-            self.cresult.write('\0')
-
     def runcommand(self):
         return super(chgcmdserver, self).runcommand()
@@ -536,5 +461,4 @@  class chgcmdserver(commandserver.server)
     capabilities.update({'attachio': attachio,
                          'chdir': chdir,
-                         'getpager': getpager,
                          'runcommand': runcommand,
                          'setenv': setenv,