diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 54ff8b3382..89bc2178d4 100644 --- a/CHANGELOG.rst +++ b/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,10 +38,15 @@ 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 ~~~~~ @@ -41,7 +54,7 @@ Fixed Reported by @jjm Contributed by Nick Maludy (Encore Technologies). - + 2.8.0 - July 10, 2018 --------------------- diff --git a/st2client/st2client/formatters/table.py b/st2client/st2client/formatters/table.py index ff783349fc..c29c583426 100644 --- a/st2client/st2client/formatters/table.py +++ b/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. diff --git a/st2client/st2client/utils/terminal.py b/st2client/st2client/utils/terminal.py index d53a342fc5..b7ca3da4cd 100644 --- a/st2client/st2client/utils/terminal.py +++ b/st2client/st2client/utils/terminal.py @@ -14,6 +14,7 @@ # limitations under the License. from __future__ import absolute_import + import os import struct import subprocess @@ -21,35 +22,61 @@ from st2client.utils.color import format_status +DEFAULT_TERMINAL_SIZE_COLUMNS = 150 + __all__ = [ - 'get_terminal_size' + 'DEFAULT_TERMINAL_SIZE_COLUMNS', + + '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, @@ -57,15 +84,11 @@ def ioctl_GWINSZ(fd): 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 diff --git a/st2client/tests/unit/test_util_terminal.py b/st2client/tests/unit/test_util_terminal.py new file mode 100644 index 0000000000..b7635f2494 --- /dev/null +++ b/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)