[PATCH] Flush password prompts for non-terminal callers

[Mads Kiilerich]

Steven Streeting wrote, On 01/15/2011 04:40 PM:
> # HG changeset patch
> # User Steve Streeting<steve at stevestreeting.com>
> # Date 1295105307 0
> # Node ID 407ab604cbb7b923f8e517e5f4858b0005bc14d0
> # Parent  8f72d2f6c26322c235602280ea0a2b3debf37b06
> Ensure that password prompts are flushed so that any non-terminal waiting for them on stderr can reliably see them. This is required to do interactive login from non-terminal clients, without forcing the user to pre-populate their configuration parameters. Without this feature, hg commands which require authentication which hasn't been pre-configured will simply randomly hang waiting for input without the calling program being any the wiser. Flushing the prompt means it always has a chance to know that hg is waiting for a response.
> This is discussed in more detail in this tracker item: http://mercurial.selenic.com/bts/issue2590

Thanks for the patch.

A meta comment: See 
http://mercurial.selenic.com/wiki/ContributingChanges , especially
* Patches should be in the '# HG changeset patch' form output by 'hg export'
* patchbomb extension always works (and makes sure the summary is 
included correctly in the patch)
* lowercase summary line, no trailing period
* all lines less than 80 characters
* a blank line after the summary
* add '(issueNNNN)' to summary to refer to a BugTracker issue

> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -468,7 +468,12 @@
>                   # windows sometimes raises something other than ImportError
>               except Exception:
>                   pass
> -        line = raw_input(prompt)
> +        # prompt using read_line instead of raw_input which ensures prompt is flushed
> +        self.write_err(prompt)
> +        line = sys.stdin.readline()
> +        # get rid of \r
> +        if line and line[-1] == '\r':
> +            line = line[:-1]

We generally like readable code without lots of comments. Comments might 
be good for describing _why_ the code does what it does if it isn't 
obvious, but they should not be used (or necessary) to describe _what_ 
the code does.

>           # When stdin is in binary mode on Windows, it can cause
>           # raw_input() to emit an extra trailing carriage return
>           if os.linesep == '\r\n' and line and line[-1] == '\r':
> @@ -508,7 +513,13 @@
>           if not self.interactive():
>               return default
>           try:
> -            return getpass.getpass(prompt or _('password: '))
> +            # Ensure we flush so that we can read prompt in non-terminal
> +            self.write_err('password:')

Don't change the prompt.

> +            pwd = sys.stdin.readline()
> +            # get rid of \r
> +            if pwd and pwd[-1] == '\r':
> +                pwd = pwd[:-1]
> +            return pwd
>           except EOFError:
>               raise util.Abort(_('response expected'))
>       def status(self, *msg, **opts):

You might have a point that there is an issue with raw_input and getpass 
not flushing after showing the prompt, but why dismiss them completely?

Instead I suggest first writing the prompt (with flushing), and then 
call the existing functions with '' as prompt.

/Mads