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

Yuya Nishihara yuya at tcha.org
Sat Oct 18 22:33:37 CDT 2014


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:
> >> On 10/10/2014 05:09 PM, Yuya Nishihara wrote:
> >>> # HG changeset patch
> >>> # User Yuya Nishihara <yuya at tcha.org>
> >>> # Date 1412950877 -32400
> >>> #      Fri Oct 10 23:21:17 2014 +0900
> >>> # Node ID 6dcb36c1781c7afbdcee20c793be20e90854e391
> >>> # Parent  80977d0b038a9a42a9b6819e77b80d343c1f711e
> >>> ui: echo prompt choice only if formatted output is acceptable
> >>>
> >>> 9ab18a912c44 is nice for test output, but it also affects command-server
> >>> clients or scripts that have to parse the output.  So enables it only if
> >>> formatted output is acceptable.
> >>>
> >>> This should cover most of command-server clients but chg.
> >>>
> >>> diff --git a/mercurial/ui.py b/mercurial/ui.py
> >>> --- a/mercurial/ui.py
> >>> +++ b/mercurial/ui.py
> >>> @@ -684,7 +684,8 @@ class ui(object):
> >>>                    r = default
> >>>                # sometimes self.interactive disagrees with isatty,
> >>>                # show response provided on stdin when simulating
> >>> -            if not self._isatty(self.fin):
> >>> +            # but not for commandserver and other scripting
> >>> +            if not self._isatty(self.fin) and self.formatted():
> >>
> >> This change is apparently on top of your change from self.isatty to
> >> self._isatty which isn't on selenic yet? I thought the change to _isatty
> >> already addressed the command server issue? Why is additional changes on
> >> top of that needed?
> >
> > self._isatty() does nothing different than util.isatty() in this case.
> > I wrote that patch just because self.interactive() calls self._isatty().
> >
> > But, it seems the original patch was already ninja queued?
> >
> >> 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?

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

Regards,


More information about the Mercurial-devel mailing list