Merge lp:~gz/brz/ui_enter into lp:brz

[Martin Packman]

Further work on sane ui factory interface and fix choose in char mode

Supersedes previous proposal:

https://code.launchpad.net/~gz/brz/ui_config_after_init/+merge/327992

But does not include requirement to initialize() before using config, instead leave ugly hack in place there for now.

[Jelmer Vernooij (jelmer)]

Thanks :)

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz-dev/76/

=== modified file 'breezy/tests/__init__.py'
--- breezy/tests/__init__.py	2018-03-16 02:56:15 +0000
+++ breezy/tests/__init__.py	2018-03-24 17:11:49 +0000
@@ -1959,11 +1959,12 @@
             os.chdir(working_dir)
 
         try:
-            result = self.apply_redirected(
-                ui.ui_factory.stdin,
-                stdout, stderr,
-                _mod_commands.run_bzr_catch_user_errors,
-                args)
+            with ui.ui_factory:
+                result = self.apply_redirected(
+                    ui.ui_factory.stdin,
+                    stdout, stderr,
+                    _mod_commands.run_bzr_catch_user_errors,
+                    args)
         finally:
             logger.removeHandler(handler)
             ui.ui_factory = old_ui_factory

=== modified file 'breezy/tests/per_uifactory/__init__.py'
--- breezy/tests/per_uifactory/__init__.py	2017-05-22 00:56:52 +0000
+++ breezy/tests/per_uifactory/__init__.py	2018-03-24 17:11:49 +0000
@@ -44,6 +44,7 @@
 from ..ui_testing import (
     StringIOWithEncoding,
     StringIOAsTTY,
+    TextUIFactory,
     )
 
 
@@ -124,11 +125,15 @@
 
     def setUp(self):
         super(TestTextUIFactory, self).setUp()
-        self.stdin = StringIOWithEncoding()
-        self.stdout = StringIOWithEncoding()
-        self.stderr = StringIOWithEncoding()
-        self.factory = ui.text.TextUIFactory(self.stdin, self.stdout,
-            self.stderr)
+        self.factory = self._create_ui_factory()
+        self.factory.__enter__()
+        self.addCleanup(self.factory.__exit__, None, None, None)
+        self.stdin = self.factory.stdin
+        self.stdout = self.factory.stdout
+        self.stderr = self.factory.stderr
+
+    def _create_ui_factory(self):
+        return TextUIFactory(u'')
 
     def _check_note(self, note_text):
         self.assertEqual("%s\n" % note_text,
@@ -172,16 +177,11 @@
 
 class TestTTYTextUIFactory(TestTextUIFactory):
 
-    def setUp(self):
-        super(TestTTYTextUIFactory, self).setUp()
-
+    def _create_ui_factory(self):
         # Remove 'TERM' == 'dumb' which causes us to *not* treat output as a
         # real terminal, even though isatty returns True
         self.overrideEnv('TERM', None)
-        self.stderr = StringIOAsTTY()
-        self.stdout = StringIOAsTTY()
-        self.factory = ui.text.TextUIFactory(self.stdin, self.stdout,
-            self.stderr)
+        return TextUIFactory(u'', StringIOAsTTY(), StringIOAsTTY())
 
     def _check_log_transport_activity_display(self):
         self.assertEqual('', self.stdout.getvalue())

=== modified file 'breezy/tests/test_ui.py'
--- breezy/tests/test_ui.py	2018-02-24 15:50:23 +0000
+++ breezy/tests/test_ui.py	2018-03-24 17:11:49 +0000
@@ -53,11 +53,13 @@
 
     def test_text_factory_confirm(self):
         # turns into reading a regular boolean
-        ui = ui_testing.TestUIFactory('n\n')
-        self.assertEqual(ui.confirm_action(u'Should %(thing)s pass?',
-            'breezy.tests.test_ui.confirmation',
-            {'thing': 'this'},),
-            False)
+        with ui_testing.TestUIFactory('n\n') as ui:
+            self.assertEqual(
+                False,
+                ui.confirm_action(
+                    u'Should %(thing)s pass?',
+                    'breezy.tests.test_ui.confirmation',
+                    {'thing': 'this'}))
 
     def test_text_factory_ascii_password(self):
         ui = ui_testing.TestUIFactory('secret\n')
@@ -93,44 +95,51 @@
             "I'm sure!\nyes\n" # True
             "NO\n" # False
             "foo\n")
-        factory = ui_testing.TextUIFactory(stdin_text)
-        self.assertEqual(True, factory.get_boolean(u""))
-        self.assertEqual(False, factory.get_boolean(u""))
-        self.assertEqual(True, factory.get_boolean(u""))
-        self.assertEqual(False, factory.get_boolean(u""))
-        self.assertEqual(True, factory.get_boolean(u""))
-        self.assertEqual(False, factory.get_boolean(u""))
-        self.assertEqual(True, factory.get_boolean(u""))
-        self.assertEqual(False, factory.get_boolean(u""))
-        self.assertEqual("foo\n", factory.stdin.read())
-        # stdin should be empty
-        self.assertEqual('', factory.stdin.readline())
-        # return false on EOF
-        self.assertEqual(False, factory.get_boolean(u""))
+        with ui_testing.TextUIFactory(stdin_text) as factory:
+            self.assertEqual(True, factory.get_boolean(u""))
+            self.assertEqual(False, factory.get_boolean(u""))
+            self.assertEqual(True, factory.get_boolean(u""))
+            self.assertEqual(False, factory.get_boolean(u""))
+            self.assertEqual(True, factory.get_boolean(u""))
+            self.assertEqual(False, factory.get_boolean(u""))
+            self.assertEqual(True, factory.get_boolean(u""))
+            self.assertEqual(False, factory.get_boolean(u""))
+            self.assertEqual("foo\n", factory.stdin.read())
+            # stdin should be empty
+            self.assertEqual('', factory.stdin.readline())
+            # return false on EOF
+            self.assertEqual(False, factory.get_boolean(u""))
 
     def test_text_ui_choose_bad_parameters(self):
-        factory = ui_testing.TextUIFactory(u"")
-        # invalid default index
-        self.assertRaises(ValueError, factory.choose, u"", u"&Yes\n&No", 3)
-        # duplicated choice
-        self.assertRaises(ValueError, factory.choose, u"", u"&choice\n&ChOiCe")
-        # duplicated shortcut
-        self.assertRaises(ValueError, factory.choose, u"", u"&choice1\nchoi&ce2")
+        with ui_testing.TextUIFactory(u"") as factory:
+            # invalid default index
+            self.assertRaises(ValueError, factory.choose, u"", u"&Yes\n&No", 3)
+            # duplicated choice
+            self.assertRaises(
+                ValueError, factory.choose, u"", u"&choice\n&ChOiCe")
+            # duplicated shortcut
+            self.assertRaises(
+                ValueError, factory.choose, u"", u"&choice1\nchoi&ce2")
 
     def test_text_ui_choose_prompt_explicit(self):
         # choices with explicit shortcuts
-        factory = ui_testing.TextUIFactory(u"")
-        factory.choose(u"prompt", u"&yes\n&No\nmore &info")
-        self.assertEqual("prompt ([y]es, [N]o, more [i]nfo): \n", factory.stderr.getvalue())
+        with ui_testing.TextUIFactory(u"") as factory:
+            factory.choose(u"prompt", u"&yes\n&No\nmore &info")
+            self.assertEqual(
+                "prompt ([y]es, [N]o, more [i]nfo): \n",
+                factory.stderr.getvalue())
 
     def test_text_ui_choose_prompt_automatic(self):
         # automatic shortcuts
-        factory = ui_testing.TextUIFactory(u"")
-        factory.choose(u"prompt", u"yes\nNo\nmore info")
-        self.assertEqual("prompt ([y]es, [N]o, [m]ore info): \n", factory.stderr.getvalue())
+        with ui_testing.TextUIFactory(u"") as factory:
+            factory.choose(u"prompt", u"yes\nNo\nmore info")
+            self.assertEqual(
+                "prompt ([y]es, [N]o, [m]ore info): \n",
+                factory.stderr.getvalue())
 
     def test_text_ui_choose_return_values(self):
-        choose = lambda: factory.choose(u"", u"&Yes\n&No\nMaybe\nmore &info", 3)
+        def choose():
+            return factory.choose(u"", u"&Yes\n&No\nMaybe\nmore &info", 3)
         stdin_text = (
             "y\n" # 0
             "n\n" # 1
@@ -142,70 +151,71 @@
             "info\nmore info\n" # 3
             "Maybe\n" # 2
             "foo\n")
-        factory = ui_testing.TextUIFactory(stdin_text)
-        self.assertEqual(0, choose())
-        self.assertEqual(1, choose())
-        self.assertEqual(3, choose())
-        self.assertEqual(1, choose())
-        self.assertEqual(0, choose())
-        self.assertEqual(1, choose())
-        self.assertEqual(3, choose())
-        self.assertEqual(2, choose())
-        self.assertEqual("foo\n", factory.stdin.read())
-        # stdin should be empty
-        self.assertEqual('', factory.stdin.readline())
-        # return None on EOF
-        self.assertEqual(None, choose())
+        with ui_testing.TextUIFactory(stdin_text) as factory:
+            self.assertEqual(0, choose())
+            self.assertEqual(1, choose())
+            self.assertEqual(3, choose())
+            self.assertEqual(1, choose())
+            self.assertEqual(0, choose())
+            self.assertEqual(1, choose())
+            self.assertEqual(3, choose())
+            self.assertEqual(2, choose())
+            self.assertEqual("foo\n", factory.stdin.read())
+            # stdin should be empty
+            self.assertEqual('', factory.stdin.readline())
+            # return None on EOF
+            self.assertEqual(None, choose())
 
     def test_text_ui_choose_no_default(self):
         stdin_text = (
             " \n" # no default, invalid!
             " yes \n" # 0
             "foo\n")
-        factory = ui_testing.TextUIFactory(stdin_text)
-        self.assertEqual(0, factory.choose(u"", u"&Yes\n&No"))
-        self.assertEqual("foo\n", factory.stdin.read())
+        with ui_testing.TextUIFactory(stdin_text) as factory:
+            self.assertEqual(0, factory.choose(u"", u"&Yes\n&No"))
+            self.assertEqual("foo\n", factory.stdin.read())
 
     def test_text_ui_get_integer(self):
         stdin_text = (
             "1\n"
             "  -2  \n"
             "hmmm\nwhat else ?\nCome on\nok 42\n4.24\n42\n")
-        factory = ui_testing.TextUIFactory(stdin_text)
-        self.assertEqual(1, factory.get_integer(u""))
-        self.assertEqual(-2, factory.get_integer(u""))
-        self.assertEqual(42, factory.get_integer(u""))
+        with ui_testing.TextUIFactory(stdin_text) as factory:
+            self.assertEqual(1, factory.get_integer(u""))
+            self.assertEqual(-2, factory.get_integer(u""))
+            self.assertEqual(42, factory.get_integer(u""))
 
     def test_text_factory_prompt(self):
         # see <https://launchpad.net/bugs/365891>
-        factory = ui_testing.TextUIFactory()
-        factory.prompt(u'foo %2e')
-        self.assertEqual('', factory.stdout.getvalue())
-        self.assertEqual('foo %2e', factory.stderr.getvalue())
+        with ui_testing.TextUIFactory() as factory:
+            factory.prompt(u'foo %2e')
+            self.assertEqual('', factory.stdout.getvalue())
+            self.assertEqual('foo %2e', factory.stderr.getvalue())
 
     def test_text_factory_prompts_and_clears(self):
         # a get_boolean call should clear the pb before prompting
         out = ui_testing.StringIOAsTTY()
         self.overrideEnv('TERM', 'xterm')
         factory = ui_testing.TextUIFactory("yada\ny\n", stdout=out, stderr=out)
-        pb = factory.nested_progress_bar()
-        pb._avail_width = lambda: 79
-        pb.show_bar = False
-        pb.show_spinner = False
-        pb.show_count = False
-        pb.update("foo", 0, 1)
-        self.assertEqual(True,
-                         self.apply_redirected(None, factory.stdout,
-                                               factory.stdout,
-                                               factory.get_boolean,
-                                               u"what do you want"))
-        output = out.getvalue()
-        self.assertContainsRe(output,
-            "| foo *\r\r  *\r*")
-        self.assertContainsString(output,
-            r"what do you want? ([y]es, [n]o): what do you want? ([y]es, [n]o): ")
-        # stdin should have been totally consumed
-        self.assertEqual('', factory.stdin.readline())
+        with factory:
+            pb = factory.nested_progress_bar()
+            pb._avail_width = lambda: 79
+            pb.show_bar = False
+            pb.show_spinner = False
+            pb.show_count = False
+            pb.update("foo", 0, 1)
+            self.assertEqual(
+                True,
+                self.apply_redirected(
+                    None, factory.stdout, factory.stdout, factory.get_boolean,
+                    u"what do you want"))
+            output = out.getvalue()
+            self.assertContainsRe(output,
+                "| foo *\r\r  *\r*")
+            self.assertContainsString(output,
+                r"what do you want? ([y]es, [n]o): what do you want? ([y]es, [n]o): ")
+            # stdin should have been totally consumed
+            self.assertEqual('', factory.stdin.readline())
 
     def test_text_tick_after_update(self):
         ui_factory = ui_testing.TextUIFactory()
@@ -236,11 +246,12 @@
         self.overrideEnv('BRZ_PROGRESS_BAR', 'text')
         ui_factory = ui_testing.TextUIFactory(
             stderr=ui_testing.StringIOAsTTY())
-        self.assertIsInstance(ui_factory._progress_view,
-            _mod_ui_text.TextProgressView)
-        ui_factory.be_quiet(True)
-        self.assertIsInstance(ui_factory._progress_view,
-            _mod_ui_text.NullProgressView)
+        with ui_factory:
+            self.assertIsInstance(ui_factory._progress_view,
+                _mod_ui_text.TextProgressView)
+            ui_factory.be_quiet(True)
+            self.assertIsInstance(ui_factory._progress_view,
+                _mod_ui_text.NullProgressView)
 
     def test_text_ui_show_user_warning(self):
         from ..bzr.groupcompress_repo import RepositoryFormat2a

=== modified file 'breezy/tests/ui_testing.py'
--- breezy/tests/ui_testing.py	2017-06-14 22:19:52 +0000
+++ breezy/tests/ui_testing.py	2018-03-24 17:11:49 +0000
@@ -62,6 +62,11 @@
             stderr = StringIOWithEncoding()
         super(TextUIFactory, self).__init__(stdin, stdout, stderr)
 
+    def _setup_streams(self):
+        self.raw_stdin = self.stdin
+        self.raw_stdout = self.stdout
+        self.raw_stderr = self.stderr
+
 
 class TestUIFactory(TextUIFactory):
     """A UI Factory for testing.

=== modified file 'breezy/ui/__init__.py'
--- breezy/ui/__init__.py	2017-05-22 00:56:52 +0000
+++ breezy/ui/__init__.py	2018-03-24 17:11:49 +0000
@@ -522,12 +522,7 @@
     """
     # this is now always TextUIFactory, which in turn decides whether it
     # should display progress bars etc
-    from .text import TextUIFactory, _wrap_in_stream, _wrap_out_stream
-    # GZ 2017-05-21: May want to rewrap streams on Python 3 if encoding config
-    if not PY3:
-        stdin = _wrap_in_stream(stdin)
-        stdout = _wrap_out_stream(stdout)
-        stderr = _wrap_out_stream(stderr)
+    from .text import TextUIFactory
     return TextUIFactory(stdin, stdout, stderr)
 
 

=== modified file 'breezy/ui/text.py'
--- breezy/ui/text.py	2017-06-14 22:19:52 +0000
+++ breezy/ui/text.py	2018-03-24 17:11:49 +0000
@@ -65,13 +65,14 @@
         variable is set to 'line-based', or if there is no controlling
         terminal.
         """
-        if os.environ.get('BRZ_TEXTUI_INPUT') != 'line-based' and \
-           self.ui.stdin == sys.stdin and self.ui.stdin.isatty():
+        is_tty = self.ui.raw_stdin.isatty()
+        if (os.environ.get('BRZ_TEXTUI_INPUT') != 'line-based' and
+                self.ui.raw_stdin == sys.stdin and is_tty):
             self.line_based = False
             self.echo_back = True
         else:
             self.line_based = True
-            self.echo_back = not self.ui.stdin.isatty()
+            self.echo_back = not is_tty
 
     def _build_alternatives(self, msg, choices, default):
         """Parse choices string.
@@ -126,7 +127,9 @@
             raise KeyboardInterrupt
         if char == chr(4): # EOF (^d, C-d)
             raise EOFError
-        return char.decode("ascii", "replace")
+        if isinstance(char, bytes):
+            return char.decode('ascii', 'replace')
+        return char
 
     def interact(self):
         """Keep asking the user until a valid choice is made.
@@ -173,8 +176,22 @@
         self.stdin = stdin
         self.stdout = stdout
         self.stderr = stderr
+        self._progress_view = NullProgressView()
+
+    def __enter__(self):
+        # Choose default encoding and handle py2/3 differences
+        self._setup_streams()
         # paints progress, network activity, etc
         self._progress_view = self.make_progress_view()
+        return self
+
+    def _setup_streams(self):
+        self.raw_stdin = _unwrap_stream(self.stdin)
+        self.stdin = _wrap_in_stream(self.raw_stdin)
+        self.raw_stdout = _unwrap_stream(self.stdout)
+        self.stdout = _wrap_out_stream(self.raw_stdout)
+        self.raw_stderr = _unwrap_stream(self.stderr)
+        self.stderr = _wrap_out_stream(self.raw_stderr)
 
     def choose(self, msg, choices, default=None):
         """Prompt the user for a list of alternatives.
@@ -642,7 +659,7 @@
 def _unwrap_stream(stream):
     inner = getattr(stream, "buffer", None)
     if inner is None:
-        inner = getattr(stream, "stream", None)
+        inner = getattr(stream, "stream", stream)
     return inner
 
 

=== modified file 'brz'
--- brz	2017-09-06 04:15:55 +0000
+++ brz	2018-03-24 17:11:49 +0000
@@ -89,14 +89,10 @@
 
 
 if __name__ == '__main__':
-    library_state = breezy.initialize()
-    library_state.__enter__()
-    try:
+    with breezy.initialize():
         exit_val = breezy.commands.main()
         if profiling:
             profile_imports.log_stack_info(sys.stderr)
-    finally:
-        library_state.__exit__(None, None, None)
 
     # By this point we really have completed everything we want to do, and
     # there's no point doing any additional cleanup.  Abruptly exiting here