From db8610ef650bd9834ab33813c2ce295c3d4e99b5 Mon Sep 17 00:00:00 2001 From: JordonPhillips Date: Wed, 18 May 2016 23:31:22 -0700 Subject: [PATCH] Print help if pager is not on PATH This checks to see if the PAGER is available on POSIX systems before attempting to run it. If it is not found, then the help information will just be printed. This adds support for the rare system that does not have `less` installed. Fixes #1957 --- .changes/next-release/bugfix-help.json | 5 +++++ awscli/help.py | 9 +++++++++ tests/unit/test_help.py | 28 ++++++++++++++++++++++---- 3 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 .changes/next-release/bugfix-help.json diff --git a/.changes/next-release/bugfix-help.json b/.changes/next-release/bugfix-help.json new file mode 100644 index 000000000000..cabf37f63a26 --- /dev/null +++ b/.changes/next-release/bugfix-help.json @@ -0,0 +1,5 @@ +{ + "description": "Write help content to stdout if less is not installed. Fixes `#1957 `__", + "category": "help", + "type": "bugfix" +} diff --git a/awscli/help.py b/awscli/help.py index 09a230a180e3..cef353767bfb 100644 --- a/awscli/help.py +++ b/awscli/help.py @@ -12,6 +12,7 @@ # language governing permissions and limitations under the License. import logging import os +import sys import platform import shlex from subprocess import Popen, PIPE @@ -62,6 +63,8 @@ class PagingHelpRenderer(object): a particular platform. """ + def __init__(self, output_stream=sys.stdout): + self.output_stream = output_stream PAGER = None @@ -114,6 +117,12 @@ def _convert_doc_content(self, contents): def _send_output_to_pager(self, output): cmdline = self.get_pager_cmdline() + if not self._exists_on_path(cmdline[0]): + LOG.debug("Pager '%s' not found in PATH, printing raw help." % + cmdline[0]) + self.output_stream.write(output.decode('utf-8') + "\n") + self.output_stream.flush() + return LOG.debug("Running command: %s", cmdline) with ignore_ctrl_c(): # We can't rely on the KeyboardInterrupt from diff --git a/tests/unit/test_help.py b/tests/unit/test_help.py index dff4c8f19419..718b6820b641 100644 --- a/tests/unit/test_help.py +++ b/tests/unit/test_help.py @@ -19,6 +19,7 @@ import mock +from awscli.compat import six from awscli.help import PosixHelpRenderer, ExecutableNotFoundError from awscli.help import WindowsHelpRenderer, ProviderHelpCommand, HelpCommand from awscli.help import TopicListerCommand, TopicHelpCommand @@ -40,11 +41,15 @@ def _popen(self, *args, **kwargs): class FakePosixHelpRenderer(HelpSpyMixin, PosixHelpRenderer): - pass + def __init__(self, output_stream=sys.stdout): + HelpSpyMixin.__init__(self) + PosixHelpRenderer.__init__(self, output_stream) class FakeWindowsHelpRenderer(HelpSpyMixin, WindowsHelpRenderer): - pass + def __init__(self, output_stream=sys.stdout): + HelpSpyMixin.__init__(self) + WindowsHelpRenderer.__init__(self, output_stream) class TestHelpPager(unittest.TestCase): @@ -96,10 +101,23 @@ def test_pager_with_args(self): def test_no_groff_exists(self): renderer = FakePosixHelpRenderer() renderer.exists_on_path['groff'] = False - with self.assertRaisesRegexp(ExecutableNotFoundError, - 'Could not find executable named "groff"'): + expected_error = 'Could not find executable named "groff"' + with self.assertRaisesRegexp(ExecutableNotFoundError, expected_error): renderer.render('foo') + @skip_if_windows('Requires POSIX system.') + def test_no_pager_exists(self): + fake_pager = 'foobar' + os.environ['MANPAGER'] = fake_pager + stdout = six.StringIO() + renderer = FakePosixHelpRenderer(output_stream=stdout) + renderer.exists_on_path[fake_pager] = False + + renderer.exists_on_path['groff'] = True + renderer.mock_popen.communicate.return_value = (b'foo', '') + renderer.render('foo') + self.assertEqual(stdout.getvalue(), 'foo\n') + def test_shlex_split_for_pager_var(self): pager_cmd = '/bin/sh -c "col -bx | vim -c \'set ft=man\' -"' os.environ['PAGER'] = pager_cmd @@ -109,6 +127,7 @@ def test_shlex_split_for_pager_var(self): def test_can_render_contents(self): renderer = FakePosixHelpRenderer() renderer.exists_on_path['groff'] = True + renderer.exists_on_path['less'] = True renderer.mock_popen.communicate.return_value = ('rendered', '') renderer.render('foo') self.assertEqual(renderer.popen_calls[-1][0], (['less', '-R'],)) @@ -133,6 +152,7 @@ def _is_pager_call(self, args): renderer = CtrlCRenderer() renderer.mock_popen.communicate.return_value = ('send to pager', '') renderer.exists_on_path['groff'] = True + renderer.exists_on_path['less'] = True renderer.render('foo') last_call = renderer.mock_popen.communicate.call_args_list[-1] self.assertEqual(last_call, mock.call(input='send to pager'))