Submitter | Mads Kiilerich |
---|---|
Date | Oct. 20, 2014, 3:02 p.m. |
Message ID | <544523EB.4090705@kiilerich.com> |
Download | mbox | patch |
Permalink | /patch/6428/ |
State | Deferred |
Headers | show |
Comments
On Mon, 2014-10-20 at 17:02 +0200, Mads Kiilerich wrote: > On 10/20/2014 04:48 PM, Yuya Nishihara wrote: > > On Sun, 19 Oct 2014 19:45:06 -0700, Pierre-Yves David wrote: > >> On 10/18/2014 08:33 PM, Yuya Nishihara wrote: > >>> On Sat, 18 Oct 2014 14:59:45 -0700, Pierre-Yves David wrote: > >>>> On 10/10/2014 10:53 PM, Yuya Nishihara wrote: > >>>>> On Fri, 10 Oct 2014 19:17:41 +0200, Mads Kiilerich wrote: > >> […] > >>>>>> I liked the idea of automagically showing the selected option in the > >>>>>> right cases. I do not so much like to piggy-back on ui.formatted and > >>>>>> then explicitly set it all the places where we needed. Then it would be > >>>>>> better to add a ui.shownonttypromptchoice configuration option ;-) > >>>>>> > >>>>>> Other tools scripting Mercurial might want to see the chosen output, > >>>>>> just like the test suite. I agree that other tools doing the same thing > >>>>>> (such as command server) don't want it. It would perhaps not be so bad > >>>>>> to have a separate option for controlling this behaviour ... but I > >>>>>> really like to have it this way in the test suite. > >>>>> Right. Some tools will want to see the choice + '\n', > >>>>> > >>>>> $ yes | hg xxx > output.log > >>>>> > >>>>> but others won't > >>>>> > >>>>> p = Popen('hg xxx', stdin=PIPE, stdout=PIPE) > >>>>> do_some_interactive_thing(p) > >>>>> ... > >>>> +1 for a separated option. we need to find a way to explicitly turn it > >>>> on for test (but only weakly so that commandserver test does not break) > >>> You mean? > >>> > >>> 1. add ui.shownonttypromptchoice which is _off_ by default > >>> 2. enable it in our tests globally, but not for test-commandserver.t > >>> > >>> Should I send the patch for stable? > >> Maybe. but this sound a bit clumsy. What are the use case for this echo > >> thing? test readability and log? > >> > >>> The current tip might break scripts that do some interaction programmatically > >>> without using commandserver. > >>> > >>> p = Popen('hg xxx', stdin=PIPE, stdout=PIPE) > >>> do_some_interactive_thing(p) # unwanted echo-back message might break > >>> # output parsing > >> This sounds like a regression. And we should probably fill an urgent > >> ticket to not loose track of this before the final release. I'm adding > >> mads kiilerich (responsible for initial patches) and Matt Mackall (great > >> priest of the backward compatibility) to this thread. > > Filed as > > http://bz.selenic.com/show_bug.cgi?id=4417 > > > > I'll send patches soon. > > I agree that something like > > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -713,7 +713,3 @@ class ui(object): > r = default > - # sometimes self.interactive disagrees with isatty, > - # show response provided on stdin when simulating > - # but commandserver > - if (not util.isatty(self.fin) > - and not self.configbool('ui', 'nontty')): > + if self.configbool('ui', 'promptecho'): > self.write(r, "\n") > --- a/tests/run-tests.py > +++ b/tests/run-tests.py > @@ -682,2 +682,3 @@ class Test(unittest.TestCase): > hgrc.write('interactive = False\n') > + hgrc.write('promptecho = On\n') > hgrc.write('mergemarkers = detailed\n') > > probably would be better. Exactly. I'll go ahead and queue this fix.
Patch
--- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -713,7 +713,3 @@ class ui(object): r = default - # sometimes self.interactive disagrees with isatty, - # show response provided on stdin when simulating - # but commandserver - if (not util.isatty(self.fin) - and not self.configbool('ui', 'nontty')): + if self.configbool('ui', 'promptecho'): self.write(r, "\n") --- a/tests/run-tests.py +++ b/tests/run-tests.py @@ -682,2 +682,3 @@ class Test(unittest.TestCase): hgrc.write('interactive = False\n') + hgrc.write('promptecho = On\n') hgrc.write('mergemarkers = detailed\n')