[MERGE] Add a get_login method to UIFactory and test get_password

[Martin Pool]

On 19 Feb 2007, Vincent Ladeuil <v.ladeuil+lp at free.fr> wrote:
> I needed to write tests involving querying user for login and
> password.
> 
> I rewrote part of the tests in the process so that they can be
> run from a dumb terminal (I get tired of
> test_text_factory_prompts_and_clears failures).

That sounds reasonable.  Does anything call these already or are they
new?

> Unicode experts comments welcome.
> 

> One point I couldn't decide was: should the login and passwords
> be encoded ?

I'm not sure.  I think your position is a reasonable place to start.

> I have no idea of what can be encountered as valid characters for
> login and passwords on all supported client systems and on all
> supported servers (restricting to ASCII seems clearly wrong,
> forcing utf-8 too, using unicode a joke ;). 
> 
> I thought that the safest was to not encode anything so that the
> servers receive exactly what the users typed.

> === modified file bzrlib/tests/__init__.py
> --- bzrlib/tests/__init__.py
> +++ bzrlib/tests/__init__.py
> @@ -530,6 +530,22 @@
>              return setattr(self._cstring, name, val)
>  
>  
> +class FakeStdin(StringIOWrapper):
> +    """Simulated stdin for tests only.
> +
> +    We pretend to be the real stdin by redirecting the fileno method so that
> +    getpas.getpass can succeed changing the echo mode of the real stdin. That
> +    allows tests to can user inputs without having to implement a
> +    full-fledged stdin.
> +    """

s/getpas/

Thanks for documenting this, it would have been a bit confusing.

Exposing it to the real stdin is a bit ugly but perhaps pragmatic.  
Would it work to instead open /dev/null or nul:?

> +
> +    fileno = sys.stdin.fileno
> +
> +    def __init__(self, string, encoding='ascii'):
> +        StringIOWrapper.__init__(self, string.encode(encoding))
> +        self.encoding = encoding
> +
> +
>  class TestCase(unittest.TestCase):
>      """Base class for bzr unit tests.
>      
> 
 
> === modified file bzrlib/ui/__init__.py
> --- bzrlib/ui/__init__.py
> +++ bzrlib/ui/__init__.py
> @@ -56,6 +56,21 @@
>          """See UIFactory.nested_progress_bar()."""
>          raise NotImplementedError(self.progress_bar)
>  
> +    def get_login(self, prompt='', **kwargs):
> +        """Prompt the user for a login (generally on a remote host).
> +
> +        :param prompt: The prompt to present the user
> +        :param kwargs: Arguments which will be expanded into the prompt.
> +                       This lets front ends display different things if
> +                       they so choose.
> +

Rather than passing the prompt and kwargs to these methods why not let
the caller combine them?

> +        :return: The user string, return None if the user canceled the
> +                 request. Note that we do not touch the encoding, users may
> +                 have whatever they see fit and the password should be
> +                 transported as is. 
> +        """
> +        raise NotImplementedError(self.progress_bar)

Should be self.get_login.

> +
>      def get_password(self, prompt='', **kwargs):
>          """Prompt the user for a password.
>  
> @@ -63,11 +78,14 @@
>          :param kwargs: Arguments which will be expanded into the prompt.
>                         This lets front ends display different things if
>                         they so choose.
> -        :return: The password string, return None if the user 
> -                 canceled the request.
> +
> +        :return: The password string, return None if the user canceled the
> +                 request. Note that we do not touch the encoding, users may
> +                 have whatever they see fit and the password should be
> +                 transported as is.
>          """
>          raise NotImplementedError(self.get_password)
> -        
> +
>      def nested_progress_bar(self):
>          """Return a nested progress bar.
>  
> @@ -104,7 +122,7 @@
>          self.clear_term()
>          # FIXME: make a regexp and handle case variations as well.
>          while True:
> -            self.prompt(prompt)
> +            self.prompt(prompt + "? [y/n]: ")
>              line = self.stdin.readline()
>              if line in ('y\n', 'yes\n'):
>                  return True
> @@ -126,6 +144,9 @@
>          """See UIFactory.nested_progress_bar()."""
>          return progress.DummyProgress()
>  
> +    def get_login(self, prompt='', **kwargs):
> +        return None
> +
>      def get_password(self, prompt='', **kwargs):
>          return None
>  
> 
> === modified file bzrlib/ui/text.py
> --- bzrlib/ui/text.py
> +++ bzrlib/ui/text.py
> @@ -27,6 +27,7 @@
>  
>  from bzrlib import (
>      progress,
> +    osutils,
>      )
>  """)
>  
> @@ -63,7 +64,7 @@
>  
>      def prompt(self, prompt):
>          """Emit prompt on the CLI."""
> -        self.stdout.write(prompt + "? [y/n]:")
> +        self.stdout.write(prompt)
>          
>      @deprecated_method(zero_eight)
>      def progress_bar(self):
> @@ -72,6 +73,23 @@
>          # bar depending on what we think of the terminal
>          return progress.ProgressBar()
>  
> +    def get_login(self, prompt='', **kwargs):
> +        """Prompt the user for a login (generally on a remote host).
> +
> +        :param prompt: The prompt to present the user
> +        :param kwargs: Arguments which will be expanded into the prompt.
> +                       This lets front ends display different things if
> +                       they so choose.
> +        :return: The user string, return None if the user 
> +                 canceled the request.
> +        """
> +        prompt += ': '
> +        prompt = (prompt % kwargs).encode(sys.stdout.encoding, 'replace')
> +        self.prompt(prompt)
> +        login = self.stdin.readline()
> +        login = login.rstrip('\n')
> +        return login
> +
>      def get_password(self, prompt='', **kwargs):
>          """Prompt the user for a password.
>  
> @@ -82,8 +100,8 @@
>          :return: The password string, return None if the user 
>                   canceled the request.
>          """
> +        prompt += ': '
>          prompt = (prompt % kwargs).encode(sys.stdout.encoding, 'replace')
> -        prompt += ': '
>          # There's currently no way to say 'i decline to enter a password'
>          # as opposed to 'my password is empty' -- does it matter?
>          return getpass.getpass(prompt)
> 
> === modified directory  // last-changed:v.ladeuil+lp at free.fr-20070219210827-qcz
> ... 46is44zdpfjxr
> # revision id: v.ladeuil+lp at free.fr-20070219210827-qcz46is44zdpfjxr
> # sha1: 53d122db5a837569f66e3151387997145bf67446
> # inventory sha1: 7201b85efd3244af16f0abdd4455a5bb7989f717
> # parent ids:
> #   pqm at pqm.ubuntu.com-20070217025822-306d98c244b53b08
> # base id: pqm at pqm.ubuntu.com-20070217025822-306d98c244b53b08
> # properties:
> #   branch-nick: fakestdin
> 

-- 
Martin