Rev 4751: Add tests, introduce explicit default values, always respect COLUMNS. in file:///home/vila/src/bzr/bugs/353370-notty-no-term-width/

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Oct 23 11:41:15 BST 2009


At file:///home/vila/src/bzr/bugs/353370-notty-no-term-width/

------------------------------------------------------------
revno: 4751
revision-id: v.ladeuil+lp at free.fr-20091023104115-ytq42i3b420x6j5x
parent: v.ladeuil+lp at free.fr-20091022151959-4nk4w3g0cvt2555n
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 353370-notty-no-term-width
timestamp: Fri 2009-10-23 12:41:15 +0200
message:
  Add tests, introduce explicit default values, always respect COLUMNS.
  
  * bzrlib/tests/test_osutils.py:
  (TestTerminalWidth): Test the known cases.
  
  * bzrlib/osutils.py:
  (terminal_width): Add default values and always respect the
  COLUMNS envrionment variable. Catch TIOCGWINSZ related errors more
  broadly.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2009-10-22 08:40:35 +0000
+++ b/NEWS	2009-10-23 10:41:15 +0000
@@ -22,9 +22,12 @@
 Bug Fixes
 *********
 
-* ``osutils.terminal_width()`` will now return a very large value
-  to avoid truuncated lines when using pagers (for example).
-  (Joke de Buhr, Vincent Ladeuil, #353370)
+* ``osutils.terminal_width()`` obeys the COLUMNS envrionment
+  variable and then default to a reasonable value if the terminal
+  is not a tty (when output is redirected for example). Also
+  fixes its usage under OSes that doesn't provide
+  termios.TIOCGWINSZ.
+  (Joke de Buhr, Vincent Ladeuil, #353370, #62539)
 
 Improvements
 ************

=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py	2009-10-22 15:19:59 +0000
+++ b/bzrlib/osutils.py	2009-10-23 10:41:15 +0000
@@ -1294,34 +1294,44 @@
     normalized_filename = _inaccessible_normalized_filename
 
 
+default_tty_width = 80
+"""The default terminal width for ttys."""
+default_non_tty_width = 256
+"""The default terminal width for non-ttys."""
 def terminal_width():
     """Return estimated terminal width."""
+
+    # If the env var is set, take it, user is always right
+    try:
+        return int(os.environ['COLUMNS'])
+    except (KeyError, ValueError):
+        pass
+
     isatty = getattr(sys.stdout, 'isatty', None)
     if  isatty is None or not isatty():
-        # If it's not a tty, the width makes no sense. We just use a value bug
-        # enough to avoid truncations. When the output is redirected, the
-        # pagers can then handle that themselves. A cleaner implementation
-        # would be to fix the callers to not try to format at all in these
-        # circumstances.
-        return 65536
+        # If it's not a tty, there is no way to acquire a value
+        # automatically. Yet, bzrlib expect a reasonable value here since it's
+        # used for both truncating lines or filling them. As such we need to
+        # use a reasonable default.  When the output is redirected, the pagers
+        # can then handle that themselves (or set COLUMNS if needed). A cleaner
+        # implementation would be to fix the callers to not try to format at
+        # all in these circumstances, but not formmatting when filling lines
+        # does not always make sense.
+        return default_non_tty_width
 
     if sys.platform == 'win32':
-        return win32utils.get_console_size()[0]
-    width = 0
+        return win32utils.get_console_size(default_tty_width)[0]
+
     try:
         import struct, fcntl, termios
         s = struct.pack('HHHH', 0, 0, 0, 0)
         x = fcntl.ioctl(1, termios.TIOCGWINSZ, s)
         width = struct.unpack('HHHH', x)[1]
-    except IOError:
-        pass
-    if width <= 0:
-        try:
-            width = int(os.environ['COLUMNS'])
-        except:
-            pass
-    if width <= 0:
-        width = 80
+    except (IOError, AttributeError):
+        width = -1
+
+    if width <= 0:
+        width = default_tty_width
 
     return width
 

=== modified file 'bzrlib/tests/blackbox/test_ignore.py'
--- a/bzrlib/tests/blackbox/test_ignore.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/blackbox/test_ignore.py	2009-10-23 10:41:15 +0000
@@ -32,7 +32,6 @@
 from bzrlib.errors import BzrCommandError
 from bzrlib.osutils import (
     pathjoin,
-    terminal_width,
     )
 from bzrlib.tests.test_sftp_transport import TestCaseWithSFTPServer
 from bzrlib.tests.blackbox import ExternalBase

=== modified file 'bzrlib/tests/test_osutils.py'
--- a/bzrlib/tests/test_osutils.py	2009-10-15 04:01:26 +0000
+++ b/bzrlib/tests/test_osutils.py	2009-10-23 10:41:15 +0000
@@ -23,6 +23,7 @@
 import socket
 import stat
 import sys
+import termios
 import time
 
 from bzrlib import (
@@ -1880,3 +1881,45 @@
             r"bzr: warning: some compiled extensions could not be loaded; "
             "see <https://answers\.launchpad\.net/bzr/\+faq/703>\n"
             )
+
+
+class TestTerminalWidth(tests.TestCase):
+
+    def test_default_values(self):
+        self.assertEquals(80, osutils.default_tty_width)
+        self.assertEquals(256, osutils.default_non_tty_width)
+
+    def test_defaults_to_COLUMNS(self):
+        # COLUMNS is set by the test framework
+        self.assertEquals('80', os.environ['COLUMNS'])
+        os.environ['COLUMNS'] = '12'
+        self.assertEquals(12, osutils.terminal_width())
+
+    def test_tty_default_without_columns(self):
+        del os.environ['COLUMNS']
+        self.assertEquals(osutils.default_tty_width, osutils.terminal_width())
+
+    def test_non_tty_default_without_columns(self):
+        del os.environ['COLUMNS']
+        orig_stdout = sys.stdout
+        def restore():
+            sys.stdout = orig_stdout
+        self.addCleanup(restore)
+        sys.stdout = None
+        self.assertEquals(osutils.default_non_tty_width,
+                          osutils.terminal_width())
+
+    def test_TIOCGWINSZ(self):
+        # bug 63539 is about a termios without TIOCGWINSZ attribute
+        exist = True
+        try:
+            orig = termios.TIOCGWINSZ
+        except AttributeError:
+            exist = False
+
+        def restore():
+            if exist:
+                termios.TIOCGWINSZ = orig
+        self.addCleanup(restore)
+        del termios.TIOCGWINSZ
+        self.assertEquals(osutils.default_tty_width, osutils.terminal_width())



More information about the bazaar-commits mailing list