[PATCH 4 of 5 V2] ui: echo prompt choice only if formatted output is acceptable

[Mads Kiilerich]

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.

/Mads