Merge lp:~mbp/bzr/339385-set-progress-bar into lp:~bzr/bzr/trunk-old

[Martin Pool]

[Martin Pool (mbp)]

This fixes this annoying bug that support for turning off progress bars regressed.

It also removes an over-specific error message; I doubt users really want bzr to fail if it won't support their progress bar preference, and it's even less likely code will want to specifically catch that case.

[Robert Collins (lifeless)]

This looks fine.
However dropping the exception is an API break; please update the
minimum api version in __init__.py accordingly.

 review needsfixing

review: Needs Fixing

=== modified file 'NEWS'
--- NEWS	2009-06-17 02:02:44 +0000
+++ NEWS	2009-06-17 08:35:26 +0000
@@ -23,6 +23,10 @@
 * ``Branch.set_append_revisions_only`` now works with branches on a smart
   server. (Andrew Bennetts, #365865)
 
+* Progress bars are now suppressed again when the environment variable
+  ``BZR_PROGRESS_BAR`` is set to ``none``.
+  (Martin Pool, #339385)
+
 Internals
 *********
 
@@ -53,6 +57,16 @@
 * Minor clarifications to the help for End-Of-Line conversions.
   (Ian Clatworthy)
 
+API Changes
+***********
+
+* Removed overspecific error class ``InvalidProgressBarType``.
+  (Martin Pool)
+
+* The method ``ProgressView._show_transport_activity`` is now
+  ``show_transport_activity`` because it's part of the contract between
+  this class and the UI.  (Martin Pool)
+
 
 bzr 1.16rc1 "It's yesterday in California" 2009-06-11
 #####################################################
 
=== modified file 'bzrlib/__init__.py'
--- bzrlib/__init__.py	2009-06-12 08:38:52 +0000
+++ bzrlib/__init__.py	2009-06-17 08:35:26 +0000
@@ -53,7 +53,7 @@
 version_info = (1, 17, 0, 'dev', 0)
 
 # API compatibility version: bzrlib is currently API compatible with 1.15.
-api_minimum_version = (1, 15, 0)
+api_minimum_version = (1, 17, 0)
 
 def _format_version_tuple(version_info):
     """Turn a version number 2, 3 or 5-tuple into a short string.
 
=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py	2009-06-16 12:22:20 +0000
+++ bzrlib/errors.py	2009-06-17 08:35:26 +0000
@@ -2172,15 +2172,6 @@
     _fmt = "Cannot perform local-only commits on unbound branches."
 
 
-class InvalidProgressBarType(BzrError):
-
-    _fmt = ("Environment variable BZR_PROGRESS_BAR='%(bar_type)s"
-            " is not a supported type Select one of: %(valid_types)s")
-
-    def __init__(self, bar_type, valid_types):
-        BzrError.__init__(self, bar_type=bar_type, valid_types=valid_types)
-
-
 class UnsupportedOperation(BzrError):
 
     _fmt = ("The method %(mname)s is not supported on"
 
=== modified file 'bzrlib/tests/blackbox/test_diff.py'
--- bzrlib/tests/blackbox/test_diff.py	2009-03-23 14:59:43 +0000
+++ bzrlib/tests/blackbox/test_diff.py	2009-06-17 08:35:26 +0000
@@ -360,18 +360,11 @@
         # subprocess.py that we had to workaround).
         # However, if 'diff' may not be available
         self.make_example_branch()
-        orig_progress = os.environ.get('BZR_PROGRESS_BAR')
-        try:
-            os.environ['BZR_PROGRESS_BAR'] = 'none'
-            out, err = self.run_bzr_subprocess('diff -r 1 --diff-options -ub',
-                                               universal_newlines=True,
-                                               retcode=None)
-        finally:
-            if orig_progress is None:
-                del os.environ['BZR_PROGRESS_BAR']
-            else:
-                os.environ['BZR_PROGRESS_BAR'] = orig_progress
-
+        # this will be automatically restored by the base bzr test class
+        os.environ['BZR_PROGRESS_BAR'] = 'none'
+        out, err = self.run_bzr_subprocess('diff -r 1 --diff-options -ub',
+                                           universal_newlines=True,
+                                           retcode=None)
         if 'Diff is not installed on this machine' in err:
             raise TestSkipped("No external 'diff' is available")
         self.assertEqual('', err)
 
=== modified file 'bzrlib/tests/test_ui.py'
--- bzrlib/tests/test_ui.py	2009-06-10 03:56:49 +0000
+++ bzrlib/tests/test_ui.py	2009-06-17 08:35:26 +0000
@@ -39,6 +39,7 @@
     SilentUIFactory,
     )
 from bzrlib.ui.text import (
+    NullProgressView,
     TextProgressView,
     TextUIFactory,
     )
@@ -103,6 +104,25 @@
         finally:
             pb.finished()
 
+    def test_progress_construction(self):
+        """TextUIFactory constructs the right progress view.
+        """
+        os.environ['BZR_PROGRESS_BAR'] = 'none'
+        self.assertIsInstance(TextUIFactory()._progress_view,
+            NullProgressView)
+
+        os.environ['BZR_PROGRESS_BAR'] = 'text'
+        self.assertIsInstance(TextUIFactory()._progress_view,
+            TextProgressView)
+
+        os.environ['BZR_PROGRESS_BAR'] = 'text'
+        self.assertIsInstance(TextUIFactory()._progress_view,
+            TextProgressView)
+
+        del os.environ['BZR_PROGRESS_BAR']
+        self.assertIsInstance(TextUIFactory()._progress_view,
+            TextProgressView)
+
     def test_progress_note(self):
         stderr = StringIO()
         stdout = StringIO()
 
=== modified file 'bzrlib/ui/text.py'
--- bzrlib/ui/text.py	2009-04-10 19:37:20 +0000
+++ bzrlib/ui/text.py	2009-06-17 08:35:26 +0000
@@ -19,6 +19,7 @@
 """Text UI, write output to the console.
 """
 
+import os
 import sys
 import time
 import warnings
@@ -56,8 +57,8 @@
             symbol_versioning.warn(symbol_versioning.deprecated_in((1, 11, 0))
                 % "bar_type parameter")
         # paints progress, network activity, etc
-        self._progress_view = TextProgressView(self.stderr)
-
+        self._progress_view = self._make_progress_view()
+        
     def clear_term(self):
         """Prepare the terminal for output.
 
@@ -69,6 +70,12 @@
         # to clear it.  We might need to separately check for the case of
         self._progress_view.clear()
 
+    def _make_progress_view(self):
+        if os.environ.get('BZR_PROGRESS_BAR') in ('text', None, ''):
+            return TextProgressView(self.stderr)
+        else:
+            return NullProgressView()
+
     def note(self, msg):
         """Write an already-formatted message, clearing the progress bar if necessary."""
         self.clear_term()
@@ -80,7 +87,7 @@
         This may update a progress bar, spinner, or similar display.
         By default it does nothing.
         """
-        self._progress_view._show_transport_activity(transport,
+        self._progress_view.show_transport_activity(transport,
             direction, byte_count)
 
     def _progress_updated(self, task):
@@ -98,6 +105,19 @@
         self._progress_view.clear()
 
 
+class NullProgressView(object):
+    """Soak up and ignore progress information."""
+
+    def clear(self):
+        pass
+
+    def show_progress(self, task):
+        pass
+
+    def show_transport_activity(self, transport, direction, byte_count):
+        pass
+    
+
 class TextProgressView(object):
     """Display of progress bar and other information on a tty.
 
@@ -216,7 +236,7 @@
         self._last_repaint = now
         self._repaint()
 
-    def _show_transport_activity(self, transport, direction, byte_count):
+    def show_transport_activity(self, transport, direction, byte_count):
         """Called by transports via the ui_factory, as they do IO.
 
         This may update a progress bar, spinner, or similar display.