Patchwork [03,of,10] chg: drop progress.assume-tty config

login
register
mail settings
Submitter Jun Wu
Date March 2, 2016, 10:44 a.m.
Message ID <855e65accbb4519f7140.1456915445@x1c>
Download mbox | patch
Permalink /patch/13527/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 2, 2016, 10:44 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1456746205 0
#      Mon Feb 29 11:43:25 2016 +0000
# Node ID 855e65accbb4519f7140828efe6800b0b92fd5f4
# Parent  74f9fd794398e5bebb6271cea53d5e5e5bf5e590
chg: drop progress.assume-tty config

Since chgserver will just take over the fds passed from the client. It can
pass istty check and progress.assume-tty is no longer necessary.
timeless - March 2, 2016, 2:33 p.m.
Jun Wu <quark@fb.com> wrote:
> Since chgserver will just take over the fds passed from the client. It can

`Since ...` doesn't really make a sentence on its own, replace
`client.  It` with `client, it`.
Yuya Nishihara - March 2, 2016, 3:32 p.m.
On Wed, 2 Mar 2016 10:44:05 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1456746205 0
> #      Mon Feb 29 11:43:25 2016 +0000
> # Node ID 855e65accbb4519f7140828efe6800b0b92fd5f4
> # Parent  74f9fd794398e5bebb6271cea53d5e5e5bf5e590
> chg: drop progress.assume-tty config
> 
> Since chgserver will just take over the fds passed from the client. It can
> pass istty check and progress.assume-tty is no longer necessary.
       ~~~~~
       isatty

The change looks good, but the reasoning is wrong. It was necessary to go
through progress.uisetup(). We no longer need it because the progress got
into the core.

(I haven't review the other patches. I'll apply spelling fixes in flight.)
Yuya Nishihara - March 3, 2016, 2:28 p.m.
On Wed, 2 Mar 2016 10:44:05 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1456746205 0
> #      Mon Feb 29 11:43:25 2016 +0000
> # Node ID 855e65accbb4519f7140828efe6800b0b92fd5f4
> # Parent  74f9fd794398e5bebb6271cea53d5e5e5bf5e590
> chg: drop progress.assume-tty config
> 
> Since chgserver will just take over the fds passed from the client. It can
> pass istty check and progress.assume-tty is no longer necessary.

Updated the reasoning, dropped assume-tty from Makefile, and pushed to the
clowncopter, thanks.

Patch

diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
--- a/contrib/chg/chg.c
+++ b/contrib/chg/chg.c
@@ -216,8 +216,6 @@ 
 		"--daemon-postexec", "none",
 		"--pid-file", opts->pidfile,
 		"--config", "extensions.chgserver=",
-		/* wrap root ui so that it can be disabled/enabled by config */
-		"--config", "progress.assume-tty=1",
 	};
 	size_t baseargvsize = sizeof(baseargv) / sizeof(baseargv[0]);
 	size_t argsize = baseargvsize + opts->argsize + 1;
diff --git a/hgext/chgserver.py b/hgext/chgserver.py
--- a/hgext/chgserver.py
+++ b/hgext/chgserver.py
@@ -580,8 +580,6 @@ 
 
 class chgunixservice(commandserver.unixservice):
     def init(self):
-        # drop options set for "hg serve --cmdserver" command
-        self.ui.setconfig('progress', 'assume-tty', None)
         signal.signal(signal.SIGHUP, self._reloadconfig)
         self._inithashstate()
         class cls(AutoExitMixIn, SocketServer.ForkingMixIn,