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

Matt Mackall mpm at selenic.com
Mon Oct 20 14:40:03 CDT 2014


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.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list