| Submitter | Yuya Nishihara |
|---|---|
| Date | Oct. 20, 2014, 3:15 p.m. |
| Message ID | <c5b0405c9f23896ad444.1413818148@mimosa> |
| Download | mbox | patch |
| Permalink | /patch/6430/ |
| State | Accepted |
| Headers | show |
Comments
On 10/20/2014 05:15 PM, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1413694842 -32400 > # Sun Oct 19 14:00:42 2014 +0900 > # Branch stable > # Node ID c5b0405c9f23896ad4440899836f13a18e9d9390 > # Parent 7ce8b804b319af65322c0ab345f21d289f92c63f > ui: enable echo back of prompt response only in tests (issue4417) > > 9ab18a912c44 isn't always good. Some tools will want to see the prompt > choice: > > $ yes | hg xxx > output.log > > but others won't: > > p = Popen('hg xxx', stdin=PIPE, stdout=PIPE) > do_some_interactive_thing(p) > > Since it could break output parsing of existing scripts, ui.simulatettyecho > is changed to be off by default. > > diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py > --- a/mercurial/commandserver.py > +++ b/mercurial/commandserver.py > @@ -196,9 +196,6 @@ class server(object): > for ui in uis: > # any kind of interaction must use server channels > ui.setconfig('ui', 'nontty', 'true', 'commandserver') > - # echo-back message shouldn't be written because a client > - # typically have to parse the output > - ui.setconfig('ui', 'simulatettyecho', 'false', 'commandserver') > > req = dispatch.request(args[:], copiedui, self.repo, self.cin, > self.cout, self.cerr) > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -714,7 +714,7 @@ class ui(object): > # sometimes self.interactive disagrees with isatty, > # show response provided on stdin when simulating > if (not util.isatty(self.fin) > - and self.configbool('ui', 'simulatettyecho', True)): > + and self.configbool('ui', 'simulatettyecho')): It is a config setting with one specific purpose. People who ask for it should get it. I would just trust it, without considering isatty. I would prefer to name the config setting something that is more to the point about what it actually does, not so much how it could be perceived. It _is_ actually doing something, not just simulating it ;-) The new config setting should also have some brief documentation. It could be argued that this setting also should control the display of default choices when not interactive. That would perhaps be more consistent ... but not big deal. Finally, I would fold these two patches into one. /Mads
On Mon, 20 Oct 2014 17:32:14 +0200, Mads Kiilerich wrote: > On 10/20/2014 05:15 PM, Yuya Nishihara wrote: > > # HG changeset patch > > # User Yuya Nishihara <yuya@tcha.org> > > # Date 1413694842 -32400 > > # Sun Oct 19 14:00:42 2014 +0900 > > # Branch stable > > # Node ID c5b0405c9f23896ad4440899836f13a18e9d9390 > > # Parent 7ce8b804b319af65322c0ab345f21d289f92c63f > > ui: enable echo back of prompt response only in tests (issue4417) > > > > 9ab18a912c44 isn't always good. Some tools will want to see the prompt > > choice: > > > > $ yes | hg xxx > output.log > > > > but others won't: > > > > p = Popen('hg xxx', stdin=PIPE, stdout=PIPE) > > do_some_interactive_thing(p) > > > > Since it could break output parsing of existing scripts, ui.simulatettyecho > > is changed to be off by default. > > > > diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py > > --- a/mercurial/commandserver.py > > +++ b/mercurial/commandserver.py > > @@ -196,9 +196,6 @@ class server(object): > > for ui in uis: > > # any kind of interaction must use server channels > > ui.setconfig('ui', 'nontty', 'true', 'commandserver') > > - # echo-back message shouldn't be written because a client > > - # typically have to parse the output > > - ui.setconfig('ui', 'simulatettyecho', 'false', 'commandserver') > > > > req = dispatch.request(args[:], copiedui, self.repo, self.cin, > > self.cout, self.cerr) > > diff --git a/mercurial/ui.py b/mercurial/ui.py > > --- a/mercurial/ui.py > > +++ b/mercurial/ui.py > > @@ -714,7 +714,7 @@ class ui(object): > > # sometimes self.interactive disagrees with isatty, > > # show response provided on stdin when simulating > > if (not util.isatty(self.fin) > > - and self.configbool('ui', 'simulatettyecho', True)): > > + and self.configbool('ui', 'simulatettyecho')): > > It is a config setting with one specific purpose. People who ask for it > should get it. I would just trust it, without considering isatty. > > I would prefer to name the config setting something that is more to the > point about what it actually does, not so much how it could be > perceived. It _is_ actually doing something, not just simulating it ;-) Okay, I'll change it to "ui.promptecho" option that prints response no matter if it is a tty. > The new config setting should also have some brief documentation. Should it? I think it's internal flag like ui.nontty. > It could be argued that this setting also should control the display of > default choices when not interactive. That would perhaps be more > consistent ... but not big deal. It would be, but shouldn't be in stable branch. > Finally, I would fold these two patches into one. Agreed. Regards,
On 10/20/2014 06:09 PM, Yuya Nishihara wrote: > On Mon, 20 Oct 2014 17:32:14 +0200, Mads Kiilerich wrote: >> On 10/20/2014 05:15 PM, Yuya Nishihara wrote: >>> # HG changeset patch >>> # User Yuya Nishihara <yuya@tcha.org> >>> # Date 1413694842 -32400 >>> # Sun Oct 19 14:00:42 2014 +0900 >>> # Branch stable >>> # Node ID c5b0405c9f23896ad4440899836f13a18e9d9390 >>> # Parent 7ce8b804b319af65322c0ab345f21d289f92c63f >>> ui: enable echo back of prompt response only in tests (issue4417) This has apparently been pushed as 5ba11ab48fcf315943225b22601dded5cc1ad8b7 Dropping the series on patchwork.
Patch
diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py --- a/mercurial/commandserver.py +++ b/mercurial/commandserver.py @@ -196,9 +196,6 @@ class server(object): for ui in uis: # any kind of interaction must use server channels ui.setconfig('ui', 'nontty', 'true', 'commandserver') - # echo-back message shouldn't be written because a client - # typically have to parse the output - ui.setconfig('ui', 'simulatettyecho', 'false', 'commandserver') req = dispatch.request(args[:], copiedui, self.repo, self.cin, self.cout, self.cerr) diff --git a/mercurial/ui.py b/mercurial/ui.py --- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -714,7 +714,7 @@ class ui(object): # sometimes self.interactive disagrees with isatty, # show response provided on stdin when simulating if (not util.isatty(self.fin) - and self.configbool('ui', 'simulatettyecho', True)): + and self.configbool('ui', 'simulatettyecho')): self.write(r, "\n") return r except EOFError: diff --git a/tests/run-tests.py b/tests/run-tests.py --- a/tests/run-tests.py +++ b/tests/run-tests.py @@ -681,6 +681,7 @@ class Test(unittest.TestCase): hgrc.write('slash = True\n') hgrc.write('interactive = False\n') hgrc.write('mergemarkers = detailed\n') + hgrc.write('simulatettyecho = True\n') hgrc.write('[defaults]\n') hgrc.write('backout = -d "0 0"\n') hgrc.write('commit = -d "0 0"\n') diff --git a/tests/test-basic.t b/tests/test-basic.t --- a/tests/test-basic.t +++ b/tests/test-basic.t @@ -8,6 +8,7 @@ Create a repository: ui.slash=True ui.interactive=False ui.mergemarkers=detailed + ui.simulatettyecho=True $ hg init t $ cd t diff --git a/tests/test-commandserver.t b/tests/test-commandserver.t --- a/tests/test-commandserver.t +++ b/tests/test-commandserver.t @@ -5,6 +5,12 @@ #endif $ export PYTHONPATH +typical client does not want echo-back messages, so test without +ui.simulatettyecho: + + $ grep -v '^simulatettyecho ' < $HGRCPATH >> $HGRCPATH.new + $ mv $HGRCPATH.new $HGRCPATH + $ hg init repo $ cd repo @@ -178,7 +184,6 @@ check that local configs for the cached ui.mergemarkers=detailed ui.foo=bar ui.nontty=true - ui.simulatettyecho=false *** runcommand init foo *** runcommand -R foo showconfig ui defaults defaults.backout=-d "0 0" @@ -189,7 +194,6 @@ check that local configs for the cached ui.interactive=False ui.mergemarkers=detailed ui.nontty=true - ui.simulatettyecho=false $ rm -R foo
# HG changeset patch # User Yuya Nishihara <yuya@tcha.org> # Date 1413694842 -32400 # Sun Oct 19 14:00:42 2014 +0900 # Branch stable # Node ID c5b0405c9f23896ad4440899836f13a18e9d9390 # Parent 7ce8b804b319af65322c0ab345f21d289f92c63f ui: enable echo back of prompt response only in tests (issue4417) 9ab18a912c44 isn't always good. Some tools will want to see the prompt choice: $ yes | hg xxx > output.log but others won't: p = Popen('hg xxx', stdin=PIPE, stdout=PIPE) do_some_interactive_thing(p) Since it could break output parsing of existing scripts, ui.simulatettyecho is changed to be off by default.