[PATCH 2 of 2 STABLE] ui: enable echo back of prompt response only in tests (issue4417)

Mads Kiilerich mads at kiilerich.com
Mon Oct 20 10:32:14 CDT 2014


On 10/20/2014 05:15 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1413694842 -32400
> #      Sun Oct 19 14:00:42 2014 +0900
> # Branch stable
> # Node ID c5b0405c9f23896ad4440899836f13a18e9d9390
> # Parent  7ce8b804b319af65322c0ab345f21d289f92c63f
> ui: enable echo back of prompt response only in tests (issue4417)
>
> 9ab18a912c44 isn't always good.  Some tools will want to see the prompt
> choice:
>
>      $ yes | hg xxx > output.log
>
> but others won't:
>
>      p = Popen('hg xxx', stdin=PIPE, stdout=PIPE)
>      do_some_interactive_thing(p)
>
> Since it could break output parsing of existing scripts, ui.simulatettyecho
> is changed to be off by default.
>
> diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py
> --- a/mercurial/commandserver.py
> +++ b/mercurial/commandserver.py
> @@ -196,9 +196,6 @@ class server(object):
>           for ui in uis:
>               # any kind of interaction must use server channels
>               ui.setconfig('ui', 'nontty', 'true', 'commandserver')
> -            # echo-back message shouldn't be written because a client
> -            # typically have to parse the output
> -            ui.setconfig('ui', 'simulatettyecho', 'false', 'commandserver')
>   
>           req = dispatch.request(args[:], copiedui, self.repo, self.cin,
>                                  self.cout, self.cerr)
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -714,7 +714,7 @@ class ui(object):
>               # sometimes self.interactive disagrees with isatty,
>               # show response provided on stdin when simulating
>               if (not util.isatty(self.fin)
> -                and self.configbool('ui', 'simulatettyecho', True)):
> +                and self.configbool('ui', 'simulatettyecho')):

It is a config setting with one specific purpose. People who ask for it 
should get it. I would just trust it, without considering isatty.

I would prefer to name the config setting something that is more to the 
point about what it actually does, not so much how it could be 
perceived. It _is_ actually doing something, not just simulating it ;-)

The new config setting should also have some brief documentation.

It could be argued that this setting also should control the display of 
default choices when not interactive. That would perhaps be more 
consistent ... but not big deal.

Finally, I would fold these two patches into one.

/Mads



More information about the Mercurial-devel mailing list