Allow user to force terminal size used by the CLI formatters by setting environment variable

CHANGELOG.rst

@@ -11,16 +11,24 @@ Added
   The ``winrm-cmd`` runner executes Command Prompt commands remotely on Windows hosts using the
   WinRM protocol. The ``winrm-ps-cmd`` and ``winrm-ps-script`` runners execute PowerShell commands
   and scripts on remote Windows hosts using the WinRM protocol.
-  
+
   To accompany these new runners, there are two new actions ``core.winrm_cmd`` that executes remote
   Command Prompt commands along with ``core.winrm_ps_cmd`` that executes remote PowerShell commands.
   (new feature) #1636
-  
+
   Contributed by Nick Maludy (Encore Technologies).
 * Add new ``?tags``, query param filter to the ``/v1/actions`` API endpoint. This query parameter
   allows users to filter out actions based on the tag name . By default, when no filter values are
   provided, all actions are returned. (new feature) #4219
-
+* Update ``st2`` CLI to inspect ``COLUMNS`` environment variable first when determining the
+  terminal size. Previously this environment variable was checked second last (after trying to
+  retrieve terminal size using various OS specific methods and before falling back to the default
+  value).
+
+  This approach is more performant and allows user to easily overwrite the default value or value
+  returned by the operating system checks - e.g. by running ``COLUMNS=200 st2 action list``.
+  (improvement) #4242
+
 Changed
 ~~~~~~~
 
@@ -30,18 +38,23 @@ Changed
 * Migrated runners to using the ``in-requirements.txt`` pattern for "components" in the build
   system, so the ``Makefile`` correctly generates and installs runner dependencies during
   testing and packaging. (improvement) (bugfix) #4169
-  
+
   Contributed by Nick Maludy (Encore Technologies).
+* Update ``st2`` CLI to use a more sensible default terminal size for table formatting purposes if
+  we are unable to retrieve terminal size using various system-specific approaches.
+
+  Previously we would fall back to a very unfriendly default of 20 columns for a total terminal
+  width. This would cause every table column to wrap and make output impossible / hard to read.
+  (improvement) #4242
 
-
 Fixed
 ~~~~~
 
 * Fixed a bug where ``secret: true`` was not applying to full object and array trees. (bugfix) #4234
   Reported by @jjm
 
   Contributed by Nick Maludy (Encore Technologies).
-  
+
 2.8.0 - July 10, 2018
 ---------------------
 

st2client/st2client/formatters/table.py

@@ -27,7 +27,7 @@
 
 from st2client import formatters
 from st2client.utils import strutil
-from st2client.utils.terminal import get_terminal_size
+from st2client.utils.terminal import get_terminal_size_columns
 
 
 LOG = logging.getLogger(__name__)
@@ -65,7 +65,7 @@ def format(cls, entries, *args, **kwargs):
 
         if not widths and attributes:
             # Dynamically calculate column size based on the terminal size
-            lines, cols = get_terminal_size()
+            cols = get_terminal_size_columns()
 
             if attributes[0] == 'id':
                 # consume iterator and save as entries so collection is accessible later.

st2client/st2client/utils/terminal.py

@@ -14,58 +14,80 @@
 # limitations under the License.
 
 from __future__ import absolute_import
+
 import os
 import struct
 import subprocess
 import sys
 
 from st2client.utils.color import format_status
 
+DEFAULT_TERMINAL_SIZE_LINES = 80
+DEFAULT_TERMINAL_SIZE_COLUMNS = 150
+
 __all__ = [
-    'get_terminal_size'
+    'get_terminal_size_columns'
 ]
 
 
-def get_terminal_size(default=(80, 20)):
+def get_terminal_size_columns(default=DEFAULT_TERMINAL_SIZE_COLUMNS):
     """
-    :return: (lines, cols)
+    Try to retrieve COLUMNS value of terminal size using various system specific approaches.
+
+    If terminal size can't be retrieved, default value is returned.
+
+    NOTE 1: COLUMNS environment variable is checked first, if the value is not set / available,
+            other methods are tried.
+
+    :rtype: ``int``
+    :return: columns
     """
+    # 1. Try COLUMNS environment variable first like in upstream Python 3 method -
+    # https://github.com/python/cpython/blob/master/Lib/shutil.py#L1203
+    # This way it's consistent with upstream implementation. In the past, our implementation
+    # checked those variables at the end as a fall back.
+    try:
+        columns = os.environ['COLUMNS']
+        return int(columns)
+    except (KeyError, ValueError):
+        pass
+
     def ioctl_GWINSZ(fd):
         import fcntl
         import termios
+        # Return a tuple (lines, columns)
         return struct.unpack('hh', fcntl.ioctl(fd, termios.TIOCGWINSZ, '1234'))
-    # try stdin, stdout, stderr
+
+    # 2. try stdin, stdout, stderr
     for fd in (0, 1, 2):
         try:
-            return ioctl_GWINSZ(fd)
-        except:
+            return ioctl_GWINSZ(fd)[1]
+        except Exception:
             pass
-    # try os.ctermid()
+
+    # 3. try os.ctermid()
     try:
         fd = os.open(os.ctermid(), os.O_RDONLY)
         try:
-            return ioctl_GWINSZ(fd)
+            return ioctl_GWINSZ(fd)[1]
         finally:
             os.close(fd)
-    except:
+    except Exception:
         pass
-    # try `stty size`
+
+    # 4. try `stty size`
     try:
         process = subprocess.Popen(['stty', 'size'],
                                    shell=False,
                                    stdout=subprocess.PIPE,
                                    stderr=open(os.devnull, 'w'))
         result = process.communicate()
         if process.returncode == 0:
-            return tuple(int(x) for x in result[0].split())
-    except:
+            return tuple(int(x) for x in result[0].split())[1]
+    except Exception:
         pass
-    # try environment variables
-    try:
-        return tuple(int(os.getenv(var)) for var in ('LINES', 'COLUMNS'))
-    except:
-        pass
-    #  return default.
+
+    # 5. return default fallback value
     return default
 
 

st2client/tests/unit/test_util_terminal.py

@@ -0,0 +1,63 @@
+# Licensed to the StackStorm, Inc ('StackStorm') under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from __future__ import absolute_import
+
+import os
+
+import unittest2
+import mock
+
+from st2client.utils.terminal import DEFAULT_TERMINAL_SIZE_COLUMNS
+from st2client.utils.terminal import get_terminal_size_columns
+
+__all__ = [
+    'TerminalUtilsTestCase'
+]
+
+
+class TerminalUtilsTestCase(unittest2.TestCase):
+    @mock.patch.dict(os.environ, {'LINES': '111', 'COLUMNS': '222'})
+    def test_get_terminal_size_columns_columns_environment_variable_has_precedence(self):
+        # Verify that COLUMNS environment variables has precedence over other approaches
+        columns = get_terminal_size_columns()
+
+        self.assertEqual(columns, 222)
+
+    @mock.patch('struct.unpack', mock.Mock(return_value=(333, 444)))
+    def test_get_terminal_size_columns_stdout_is_used(self):
+        columns = get_terminal_size_columns()
+        self.assertEqual(columns, 444)
+
+    @mock.patch('struct.unpack', mock.Mock(side_effect=Exception('a')))
+    @mock.patch('subprocess.Popen')
+    def test_get_terminal_size_subprocess_popen_is_used(self, mock_popen):
+        mock_communicate = mock.Mock(return_value=['555 666'])
+
+        mock_process = mock.Mock()
+        mock_process.returncode = 0
+        mock_process.communicate = mock_communicate
+
+        mock_popen.return_value = mock_process
+
+        columns = get_terminal_size_columns()
+        self.assertEqual(columns, 666)
+
+    @mock.patch('struct.unpack', mock.Mock(side_effect=Exception('a')))
+    @mock.patch('subprocess.Popen', mock.Mock(side_effect=Exception('b')))
+    def test_get_terminal_size_default_values_are_used(self):
+        columns = get_terminal_size_columns()
+
+        self.assertEqual(columns, DEFAULT_TERMINAL_SIZE_COLUMNS)