[PATCH 4 of 5 V2] ui: echo prompt choice only if formatted output is acceptable
Yuya Nishihara
yuya at tcha.org
Mon Oct 20 09:48:58 CDT 2014
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.
Regards,
More information about the Mercurial-devel
mailing list