From 3eca15d4e9f3423477ecc4da1d466de22b78c89f Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Sat, 22 Feb 2020 14:32:56 -0500 Subject: [PATCH] [#1470,#1476] Use subprocess instead of os.system --- click/_compat.py | 13 ++++++++ click/_termui_impl.py | 71 ++++++++++++++++++++++++++++++++++--------- click/termui.py | 5 +-- tests/test_imports.py | 2 +- 4 files changed, 73 insertions(+), 18 deletions(-) diff --git a/click/_compat.py b/click/_compat.py index 5b5c73dd6..79061c022 100644 --- a/click/_compat.py +++ b/click/_compat.py @@ -258,6 +258,17 @@ def filename_to_ui(value): if isinstance(value, bytes): value = value.decode(get_filesystem_encoding(), 'replace') return value + + def which(cmd): + import subprocess + try: + path = subprocess.check_output( + ['which', cmd], + stderr=subprocess.STDOUT, + ) + except subprocess.CalledProcessError: + return None + return path.strip() else: text_type = str raw_input = input @@ -461,6 +472,8 @@ def filename_to_ui(value): .decode('utf-8', 'replace') return value + from shutil import which + def get_streerror(e, default=None): if hasattr(e, 'strerror'): diff --git a/click/_termui_impl.py b/click/_termui_impl.py index 62eb95d21..2087840f6 100644 --- a/click/_termui_impl.py +++ b/click/_termui_impl.py @@ -18,7 +18,7 @@ import contextlib from ._compat import _default_text_stdout, range_type, isatty, \ open_stream, strip_ansi, term_len, get_best_encoding, WIN, int_types, \ - CYGWIN + CYGWIN, which from .utils import echo from .exceptions import ClickException @@ -333,15 +333,15 @@ def pager(generator, color=None): if os.environ.get('TERM') in ('dumb', 'emacs'): return _nullpager(stdout, generator, color) if WIN or sys.platform.startswith('os2'): - return _tempfilepager(generator, 'more <', color) - if hasattr(os, 'system') and os.system('(less) 2>/dev/null') == 0: + return _pipetempfilepager(generator, 'more', color) + if which('less') is not None: return _pipepager(generator, 'less', color) import tempfile fd, filename = tempfile.mkstemp() os.close(fd) try: - if hasattr(os, 'system') and os.system('more "%s"' % filename) == 0: + if which('more') is not None: return _pipepager(generator, 'more', color) return _nullpager(stdout, generator, color) finally: @@ -399,6 +399,31 @@ def _pipepager(generator, cmd, color): def _tempfilepager(generator, cmd, color): """Page through text by invoking a program on a temporary file.""" + import subprocess + import tempfile + filename = tempfile.mktemp() + # TODO: This never terminates if the passed generator never terminates. + text = "".join(generator) + if not color: + text = strip_ansi(text) + encoding = get_best_encoding(sys.stdout) + with open_stream(filename, 'wb')[0] as f: + f.write(text.encode(encoding)) + try: + subprocess.call([cmd, filename]) + except OSError: + # Command not found + pass + finally: + os.unlink(filename) + + +def _pipetempfilepager(generator, cmd, color): + """ + Page through text by invoking a program with a temporary file redirected + as input. + """ + import subprocess import tempfile filename = tempfile.mktemp() # TODO: This never terminates if the passed generator never terminates. @@ -409,7 +434,11 @@ def _tempfilepager(generator, cmd, color): with open_stream(filename, 'wb')[0] as f: f.write(text.encode(encoding)) try: - os.system(cmd + ' "' + filename + '"') + with open_stream(filename, 'rb')[0] as f: + subprocess.call([cmd], stdin=f) + except OSError: + # Command not found + pass finally: os.unlink(filename) @@ -441,7 +470,7 @@ def get_editor(self): if WIN: return 'notepad' for editor in 'sensible-editor', 'vim', 'nano': - if os.system('which %s >/dev/null 2>&1' % editor) == 0: + if which(editor) is not None: return editor return 'vi' @@ -532,20 +561,32 @@ def _unquote_file(url): elif WIN: if locate: url = _unquote_file(url) - args = 'explorer /select,"%s"' % _unquote_file( - url.replace('"', '')) + args = ['explorer', '/select,%s' % _unquote_file(url)] else: - args = 'start %s "" "%s"' % ( - wait and '/WAIT' or '', url.replace('"', '')) - return os.system(args) + args = ['start'] + if wait: + args.append('/WAIT') + args.append('') + args.append(url) + try: + return subprocess.call(args) + except OSError: + # Command not found + return 127 elif CYGWIN: if locate: url = _unquote_file(url) - args = 'cygstart "%s"' % (os.path.dirname(url).replace('"', '')) + args = ['cygstart', os.path.dirname(url)] else: - args = 'cygstart %s "%s"' % ( - wait and '-w' or '', url.replace('"', '')) - return os.system(args) + args = ['cygstart'] + if wait: + args.append('-w') + args.append(url) + try: + return subprocess.call(args) + except OSError: + # Command not found + return 127 try: if locate: diff --git a/click/termui.py b/click/termui.py index 764132309..811fb8e90 100644 --- a/click/termui.py +++ b/click/termui.py @@ -390,10 +390,11 @@ def clear(): if not isatty(sys.stdout): return # If we're on Windows and we don't have colorama available, then we - # clear the screen by shelling out. Otherwise we can use an escape + # clear the screen by invoking `cls`. Otherwise we can use an escape # sequence. if WIN: - os.system('cls') + import subprocess + subprocess.check_call(['cls']) else: sys.stdout.write('\033[2J\033[1;1H') diff --git a/tests/test_imports.py b/tests/test_imports.py index 8e9a97df6..560673aa7 100644 --- a/tests/test_imports.py +++ b/tests/test_imports.py @@ -32,7 +32,7 @@ def tracking_import(module, locals=None, globals=None, fromlist=None, ALLOWED_IMPORTS = set([ 'weakref', 'os', 'struct', 'collections', 'sys', 'contextlib', 'functools', 'stat', 're', 'codecs', 'inspect', 'itertools', 'io', - 'threading', 'colorama', 'errno', 'fcntl', 'datetime' + 'threading', 'colorama', 'errno', 'fcntl', 'datetime', 'shutil' ]) if WIN: