diff --git a/tests/rules/test_dirty_untar.py b/tests/rules/test_dirty_untar.py index 2e1c5f07..21579c8e 100644 --- a/tests/rules/test_dirty_untar.py +++ b/tests/rules/test_dirty_untar.py @@ -1,7 +1,8 @@ import os import pytest import tarfile -from thefuck.rules.dirty_untar import match, get_new_command, side_effect +from thefuck.rules.dirty_untar import match, get_new_command, side_effect, \ + tar_extensions from tests.utils import Command @@ -32,34 +33,40 @@ def tar_error(tmpdir): return fixture -parametrize_filename = pytest.mark.parametrize('filename', [ - 'foo.tar', - 'foo.tar.gz', - 'foo.tgz']) +parametrize_extensions = pytest.mark.parametrize('ext', tar_extensions) + +# (filename as typed by the user, unquoted filename, quoted filename as per shells.quote) +parametrize_filename = pytest.mark.parametrize('filename, unquoted, quoted', [ + ('foo{}', 'foo{}', 'foo{}'), + ('foo\ bar{}', 'foo bar{}', "'foo bar{}'"), + ('"foo bar{}"', 'foo bar{}', "'foo bar{}'")]) parametrize_script = pytest.mark.parametrize('script, fixed', [ - ('tar xvf {}', 'mkdir -p foo && tar xvf {} -C foo'), - ('tar -xvf {}', 'mkdir -p foo && tar -xvf {} -C foo'), - ('tar --extract -f {}', 'mkdir -p foo && tar --extract -f {} -C foo')]) + ('tar xvf {}', 'mkdir -p {dir} && tar xvf {filename} -C {dir}'), + ('tar -xvf {}', 'mkdir -p {dir} && tar -xvf {filename} -C {dir}'), + ('tar --extract -f {}', 'mkdir -p {dir} && tar --extract -f {filename} -C {dir}')]) +@parametrize_extensions @parametrize_filename @parametrize_script -def test_match(tar_error, filename, script, fixed): - tar_error(filename) - assert match(Command(script=script.format(filename))) +def test_match(ext, tar_error, filename, unquoted, quoted, script, fixed): + tar_error(unquoted.format(ext)) + assert match(Command(script=script.format(filename.format(ext)))) +@parametrize_extensions @parametrize_filename @parametrize_script -def test_side_effect(tar_error, filename, script, fixed): - tar_error(filename) - side_effect(Command(script=script.format(filename)), None) - assert set(os.listdir('.')) == {filename, 'd'} - +def test_side_effect(ext, tar_error, filename, unquoted, quoted, script, fixed): + tar_error(unquoted.format(ext)) + side_effect(Command(script=script.format(filename.format(ext))), None) + assert set(os.listdir('.')) == {unquoted.format(ext), 'd'} +@parametrize_extensions @parametrize_filename @parametrize_script -def test_get_new_command(tar_error, filename, script, fixed): - tar_error(filename) - assert get_new_command(Command(script=script.format(filename))) == fixed.format(filename) +def test_get_new_command(ext, tar_error, filename, unquoted, quoted, script, fixed): + tar_error(unquoted.format(ext)) + assert (get_new_command(Command(script=script.format(filename.format(ext)))) + == fixed.format(dir=quoted.format(''), filename=filename.format(ext))) diff --git a/tests/rules/test_dirty_unzip.py b/tests/rules/test_dirty_unzip.py index d68e936f..9c0b4e51 100644 --- a/tests/rules/test_dirty_unzip.py +++ b/tests/rules/test_dirty_unzip.py @@ -43,6 +43,8 @@ def test_side_effect(zip_error, script): @pytest.mark.parametrize('script,fixed', [ ('unzip foo', 'unzip foo -d foo'), + (R"unzip foo\ bar.zip", R"unzip foo\ bar.zip -d 'foo bar'"), + (R"unzip 'foo bar.zip'", R"unzip 'foo bar.zip' -d 'foo bar'"), ('unzip foo.zip', 'unzip foo.zip -d foo')]) def test_get_new_command(zip_error, script, fixed): assert get_new_command(Command(script=script)) == fixed diff --git a/tests/rules/test_has_exists_script.py b/tests/rules/test_has_exists_script.py index 6a94760c..c6b4b1ed 100644 --- a/tests/rules/test_has_exists_script.py +++ b/tests/rules/test_has_exists_script.py @@ -1,17 +1,18 @@ -from mock import Mock, patch +from mock import patch from thefuck.rules.has_exists_script import match, get_new_command +from ..utils import Command def test_match(): with patch('os.path.exists', return_value=True): - assert match(Mock(script='main', stderr='main: command not found')) - assert match(Mock(script='main --help', + assert match(Command(script='main', stderr='main: command not found')) + assert match(Command(script='main --help', stderr='main: command not found')) - assert not match(Mock(script='main', stderr='')) + assert not match(Command(script='main', stderr='')) with patch('os.path.exists', return_value=False): - assert not match(Mock(script='main', stderr='main: command not found')) + assert not match(Command(script='main', stderr='main: command not found')) def test_get_new_command(): - assert get_new_command(Mock(script='main --help')) == './main --help' + assert get_new_command(Command(script='main --help')) == './main --help' diff --git a/thefuck/rules/cargo_no_command.py b/thefuck/rules/cargo_no_command.py index 76eec3cb..f1816232 100644 --- a/thefuck/rules/cargo_no_command.py +++ b/thefuck/rules/cargo_no_command.py @@ -2,14 +2,14 @@ import re from thefuck.utils import replace_argument, for_app -@for_app('cargo') +@for_app('cargo', at_least=1) def match(command): return ('No such subcommand' in command.stderr and 'Did you mean' in command.stderr) def get_new_command(command): - broken = command.script.split()[1] + broken = command.split_script[1] fix = re.findall(r'Did you mean `([^`]*)`', command.stderr)[0] return replace_argument(command.script, broken, fix) diff --git a/thefuck/rules/cd_correction.py b/thefuck/rules/cd_correction.py index 24c70067..49886973 100644 --- a/thefuck/rules/cd_correction.py +++ b/thefuck/rules/cd_correction.py @@ -33,7 +33,7 @@ def get_new_command(command): defaults to the rules of cd_mkdir. Change sensitivity by changing MAX_ALLOWED_DIFF. Default value is 0.6 """ - dest = command.script.split()[1].split(os.sep) + dest = command.split_script[1].split(os.sep) if dest[-1] == '': dest = dest[:-1] cwd = os.getcwd() diff --git a/thefuck/rules/cpp11.py b/thefuck/rules/cpp11.py index c30012ac..3cd7d728 100644 --- a/thefuck/rules/cpp11.py +++ b/thefuck/rules/cpp11.py @@ -1,7 +1,7 @@ from thefuck.utils import for_app -@for_app(['g++', 'clang++']) +@for_app('g++', 'clang++') def match(command): return ('This file requires compiler and library support for the ' 'ISO C++ 2011 standard.' in command.stderr or diff --git a/thefuck/rules/dirty_untar.py b/thefuck/rules/dirty_untar.py index 4f10e3be..2b5090f5 100644 --- a/thefuck/rules/dirty_untar.py +++ b/thefuck/rules/dirty_untar.py @@ -4,6 +4,11 @@ from thefuck import shells from thefuck.utils import for_app +tar_extensions = ('.tar', '.tar.Z', '.tar.bz2', '.tar.gz', '.tar.lz', + '.tar.lzma', '.tar.xz', '.taz', '.tb2', '.tbz', '.tbz2', + '.tgz', '.tlz', '.txz', '.tz') + + def _is_tar_extract(cmd): if '--extract' in cmd: return True @@ -14,11 +19,8 @@ def _is_tar_extract(cmd): def _tar_file(cmd): - tar_extensions = ('.tar', '.tar.Z', '.tar.bz2', '.tar.gz', '.tar.lz', - '.tar.lzma', '.tar.xz', '.taz', '.tb2', '.tbz', '.tbz2', - '.tgz', '.tlz', '.txz', '.tz') - for c in cmd.split(): + for c in cmd: for ext in tar_extensions: if c.endswith(ext): return (c, c[0:len(c) - len(ext)]) @@ -28,16 +30,17 @@ def _tar_file(cmd): def match(command): return ('-C' not in command.script and _is_tar_extract(command.script) - and _tar_file(command.script) is not None) + and _tar_file(command.split_script) is not None) def get_new_command(command): + dir = shells.quote(_tar_file(command.split_script)[1]) return shells.and_('mkdir -p {dir}', '{cmd} -C {dir}') \ - .format(dir=_tar_file(command.script)[1], cmd=command.script) + .format(dir=dir, cmd=command.script) def side_effect(old_cmd, command): - with tarfile.TarFile(_tar_file(old_cmd.script)[0]) as archive: + with tarfile.TarFile(_tar_file(old_cmd.split_script)[0]) as archive: for file in archive.getnames(): try: os.remove(file) diff --git a/thefuck/rules/dirty_unzip.py b/thefuck/rules/dirty_unzip.py index d0a1248f..fced9b37 100644 --- a/thefuck/rules/dirty_unzip.py +++ b/thefuck/rules/dirty_unzip.py @@ -1,6 +1,7 @@ import os import zipfile from thefuck.utils import for_app +from thefuck.shells import quote def _is_bad_zip(file): @@ -13,7 +14,7 @@ def _zip_file(command): # unzip [-flags] file[.zip] [file(s) ...] [-x file(s) ...] # ^ ^ files to unzip from the archive # archive to unzip - for c in command.script.split()[1:]: + for c in command.split_script[1:]: if not c.startswith('-'): if c.endswith('.zip'): return c @@ -28,7 +29,7 @@ def match(command): def get_new_command(command): - return '{} -d {}'.format(command.script, _zip_file(command)[:-4]) + return '{} -d {}'.format(command.script, quote(_zip_file(command)[:-4])) def side_effect(old_cmd, command): diff --git a/thefuck/rules/dry.py b/thefuck/rules/dry.py index 075df04f..d05fdafe 100644 --- a/thefuck/rules/dry.py +++ b/thefuck/rules/dry.py @@ -1,11 +1,13 @@ def match(command): - split_command = command.script.split() + split_command = command.split_script - return len(split_command) >= 2 and split_command[0] == split_command[1] + return (split_command + and len(split_command) >= 2 + and split_command[0] == split_command[1]) def get_new_command(command): - return command.script[command.script.find(' ')+1:] + return ' '.join(command.split_script[1:]) # it should be rare enough to actually have to type twice the same word, so # this rule can have a higher priority to come before things like "cd cd foo" diff --git a/thefuck/rules/git_branch_list.py b/thefuck/rules/git_branch_list.py index d40ef98d..5c055d33 100644 --- a/thefuck/rules/git_branch_list.py +++ b/thefuck/rules/git_branch_list.py @@ -5,7 +5,8 @@ from thefuck.specific.git import git_support @git_support def match(command): # catches "git branch list" in place of "git branch" - return command.script.split()[1:] == 'branch list'.split() + return (command.split_script + and command.split_script[1:] == 'branch list'.split()) @git_support diff --git a/thefuck/rules/git_fix_stash.py b/thefuck/rules/git_fix_stash.py index 944d37c5..4714f866 100644 --- a/thefuck/rules/git_fix_stash.py +++ b/thefuck/rules/git_fix_stash.py @@ -5,9 +5,8 @@ from thefuck.specific.git import git_support @git_support def match(command): - splited_script = command.script.split() - if len(splited_script) > 1: - return (splited_script[1] == 'stash' + if command.split_script and len(command.split_script) > 1: + return (command.split_script[1] == 'stash' and 'usage:' in command.stderr) else: return False @@ -26,12 +25,12 @@ stash_commands = ( @git_support def get_new_command(command): - stash_cmd = command.script.split()[2] + stash_cmd = command.split_script[2] fixed = utils.get_closest(stash_cmd, stash_commands, fallback_to_first=False) if fixed is not None: return replace_argument(command.script, stash_cmd, fixed) else: - cmd = command.script.split() + cmd = command.split_script[:] cmd.insert(2, 'save') return ' '.join(cmd) diff --git a/thefuck/rules/has_exists_script.py b/thefuck/rules/has_exists_script.py index 0ffc0860..c62da9e7 100644 --- a/thefuck/rules/has_exists_script.py +++ b/thefuck/rules/has_exists_script.py @@ -4,7 +4,7 @@ from thefuck.specific.sudo import sudo_support @sudo_support def match(command): - return os.path.exists(command.script.split()[0]) \ + return command.split_script and os.path.exists(command.split_script[0]) \ and 'command not found' in command.stderr diff --git a/thefuck/rules/ls_lah.py b/thefuck/rules/ls_lah.py index 54b6d651..bd1f16ba 100644 --- a/thefuck/rules/ls_lah.py +++ b/thefuck/rules/ls_lah.py @@ -3,10 +3,10 @@ from thefuck.utils import for_app @for_app('ls') def match(command): - return 'ls -' not in command.script + return command.split_script and 'ls -' not in command.script def get_new_command(command): - command = command.script.split(' ') + command = command.split_script[:] command[0] = 'ls -lah' return ' '.join(command) diff --git a/thefuck/rules/man.py b/thefuck/rules/man.py index 84427e79..18972be9 100644 --- a/thefuck/rules/man.py +++ b/thefuck/rules/man.py @@ -1,5 +1,9 @@ +from thefuck.utils import for_app + + +@for_app('man', at_least=1) def match(command): - return command.script.strip().startswith('man ') + return True def get_new_command(command): @@ -8,7 +12,7 @@ def get_new_command(command): if '2' in command.script: return command.script.replace("2", "3") - split_cmd2 = command.script.split() + split_cmd2 = command.split_script split_cmd3 = split_cmd2[:] split_cmd2.insert(1, ' 2 ') diff --git a/thefuck/rules/mercurial.py b/thefuck/rules/mercurial.py index c8290fb5..b1d93de6 100644 --- a/thefuck/rules/mercurial.py +++ b/thefuck/rules/mercurial.py @@ -21,7 +21,7 @@ def match(command): def get_new_command(command): - script = command.script.split(' ') + script = command.split_script[:] possibilities = extract_possibilities(command) script[1] = get_closest(script[1], possibilities) return ' '.join(script) diff --git a/thefuck/rules/no_command.py b/thefuck/rules/no_command.py index 5318568c..2f46bfc5 100644 --- a/thefuck/rules/no_command.py +++ b/thefuck/rules/no_command.py @@ -5,16 +5,17 @@ from thefuck.specific.sudo import sudo_support @sudo_support def match(command): - return 'not found' in command.stderr and \ - bool(get_close_matches(command.script.split(' ')[0], - get_all_executables())) + return (command.split_script + and 'not found' in command.stderr + and bool(get_close_matches(command.split_script[0], + get_all_executables()))) @sudo_support def get_new_command(command): - old_command = command.script.split(' ')[0] + old_command = command.split_script[0] new_cmds = get_close_matches(old_command, get_all_executables(), cutoff=0.1) - return [' '.join([new_command] + command.script.split(' ')[1:]) + return [' '.join([new_command] + command.split_script[1:]) for new_command in new_cmds] diff --git a/thefuck/rules/pacman_not_found.py b/thefuck/rules/pacman_not_found.py index 9875935b..129f80f4 100644 --- a/thefuck/rules/pacman_not_found.py +++ b/thefuck/rules/pacman_not_found.py @@ -11,12 +11,14 @@ from thefuck.specific.archlinux import get_pkgfile, archlinux_env def match(command): - return (command.script.startswith(('pacman', 'sudo pacman', 'yaourt')) + return (command.split_script + and (command.split_script[0] in ('pacman', 'yaourt') + or command.split_script[0:2] == ['sudo', 'pacman']) and 'error: target not found:' in command.stderr) def get_new_command(command): - pgr = command.script.split()[-1] + pgr = command.split_script[-1] return replace_command(command, pgr, get_pkgfile(pgr)) diff --git a/thefuck/rules/python_command.py b/thefuck/rules/python_command.py index 95bbf021..714d6d31 100644 --- a/thefuck/rules/python_command.py +++ b/thefuck/rules/python_command.py @@ -6,8 +6,8 @@ from thefuck.specific.sudo import sudo_support @sudo_support def match(command): - toks = command.script.split() - return (len(toks) > 0 + toks = command.split_script + return (toks and toks[0].endswith('.py') and ('Permission denied' in command.stderr or 'command not found' in command.stderr)) diff --git a/thefuck/rules/rm_root.py b/thefuck/rules/rm_root.py index 28e522f5..ad4aa7e4 100644 --- a/thefuck/rules/rm_root.py +++ b/thefuck/rules/rm_root.py @@ -5,7 +5,8 @@ enabled_by_default = False @sudo_support def match(command): - return ({'rm', '/'}.issubset(command.script.split()) + return (command.split_script + and {'rm', '/'}.issubset(command.split_script) and '--no-preserve-root' not in command.script and '--no-preserve-root' in command.stderr) diff --git a/thefuck/rules/sed_unterminated_s.py b/thefuck/rules/sed_unterminated_s.py index 718d6940..1fb6233d 100644 --- a/thefuck/rules/sed_unterminated_s.py +++ b/thefuck/rules/sed_unterminated_s.py @@ -1,5 +1,6 @@ import shlex -from thefuck.utils import quote, for_app +from thefuck.shells import quote +from thefuck.utils import for_app @for_app('sed') diff --git a/thefuck/rules/switch_lang.py b/thefuck/rules/switch_lang.py index 8952eb05..23f1422d 100644 --- a/thefuck/rules/switch_lang.py +++ b/thefuck/rules/switch_lang.py @@ -11,9 +11,11 @@ source_layouts = [u'''йцукенгшщзхъфывапролджэячсмит @memoize def _get_matched_layout(command): + # don't use command.split_script here because a layout mismatch will likely + # result in a non-splitable sript as per shlex + cmd = command.script.split(' ') for source_layout in source_layouts: - if all([ch in source_layout or ch in '-_' - for ch in command.script.split(' ')[0]]): + if all([ch in source_layout or ch in '-_' for ch in cmd[0]]): return source_layout diff --git a/thefuck/rules/systemctl.py b/thefuck/rules/systemctl.py index 771206d7..2605b625 100644 --- a/thefuck/rules/systemctl.py +++ b/thefuck/rules/systemctl.py @@ -8,15 +8,15 @@ from thefuck.utils import for_app @sudo_support @for_app('systemctl') def match(command): - # Catches 'Unknown operation 'service'.' when executing systemctl with + # Catches "Unknown operation 'service'." when executing systemctl with # misordered arguments - cmd = command.script.split() - return ('Unknown operation \'' in command.stderr and + cmd = command.split_script + return (cmd and 'Unknown operation \'' in command.stderr and len(cmd) - cmd.index('systemctl') == 3) @sudo_support def get_new_command(command): - cmd = command.script.split() + cmd = command.split_script cmd[-1], cmd[-2] = cmd[-2], cmd[-1] return ' '.join(cmd) diff --git a/thefuck/rules/vagrant_up.py b/thefuck/rules/vagrant_up.py index 44093972..d6385273 100644 --- a/thefuck/rules/vagrant_up.py +++ b/thefuck/rules/vagrant_up.py @@ -8,13 +8,13 @@ def match(command): def get_new_command(command): - cmds = command.script.split(' ') + cmds = command.split_script machine = None if len(cmds) >= 3: machine = cmds[2] startAllInstances = shells.and_("vagrant up", command.script) - if machine is None: + if machine is None: return startAllInstances else: return [ shells.and_("vagrant up " + machine, command.script), startAllInstances] diff --git a/thefuck/rules/whois.py b/thefuck/rules/whois.py index 141c7332..61711de9 100644 --- a/thefuck/rules/whois.py +++ b/thefuck/rules/whois.py @@ -1,7 +1,9 @@ # -*- encoding: utf-8 -*- from six.moves.urllib.parse import urlparse +from thefuck.utils import for_app +@for_app('whois', at_least=1) def match(command): """ What the `whois` command returns depends on the 'Whois server' it contacted @@ -19,11 +21,11 @@ def match(command): - www.google.fr → subdomain: www, domain: 'google.fr'; - google.co.uk → subdomain: None, domain; 'google.co.uk'. """ - return 'whois ' in command.script.strip() + return True def get_new_command(command): - url = command.script.split()[1] + url = command.split_script[1] if '/' in command.script: return 'whois ' + urlparse(url).netloc diff --git a/thefuck/shells.py b/thefuck/shells.py index 0bbe0117..da5f95cb 100644 --- a/thefuck/shells.py +++ b/thefuck/shells.py @@ -1,6 +1,6 @@ """Module with shell specific actions, each shell class should -implement `from_shell`, `to_shell`, `app_alias`, `put_to_history` and `get_aliases` -methods. +implement `from_shell`, `to_shell`, `app_alias`, `put_to_history` and +`get_aliases` methods. """ from collections import defaultdict @@ -9,6 +9,8 @@ from subprocess import Popen, PIPE from time import time import io import os +import shlex +import six from .utils import DEVNULL, memoize, cache @@ -75,6 +77,20 @@ class Generic(object): def how_to_configure(self): return + def split_command(self, command): + """Split the command using shell-like syntax.""" + return shlex.split(command) + + def quote(self, s): + """Return a shell-escaped version of the string s.""" + + if six.PY2: + from pipes import quote + else: + from shlex import quote + + return quote(s) + class Bash(Generic): def app_alias(self, fuck): @@ -285,9 +301,18 @@ def get_aliases(): return list(_get_shell().get_aliases().keys()) +def split_command(command): + return _get_shell().split_command(command) + + +def quote(s): + return _get_shell().quote(s) + + @memoize def get_history(): return list(_get_shell().get_history()) + def how_to_configure(): return _get_shell().how_to_configure() diff --git a/thefuck/specific/git.py b/thefuck/specific/git.py index 51a81ef4..a3654cc3 100644 --- a/thefuck/specific/git.py +++ b/thefuck/specific/git.py @@ -1,8 +1,7 @@ import re -from shlex import split from decorator import decorator -from ..types import Command -from ..utils import quote, is_app +from ..utils import is_app +from ..shells import quote, split_command @decorator @@ -24,7 +23,7 @@ def git_support(fn, command): # 'commit' '--amend' # which is surprising and does not allow to easily test for # eg. 'git commit' - expansion = ' '.join(map(quote, split(search.group(2)))) + expansion = ' '.join(map(quote, split_command(search.group(2)))) new_script = command.script.replace(alias, expansion) command = command.update(script=new_script) diff --git a/thefuck/types.py b/thefuck/types.py index 44f6aeeb..779829bd 100644 --- a/thefuck/types.py +++ b/thefuck/types.py @@ -1,13 +1,13 @@ -from imp import load_source -import os -from subprocess import Popen, PIPE -import sys -from psutil import Process, TimeoutExpired -import six -from .conf import settings, DEFAULT_PRIORITY, ALL_ENABLED -from .utils import compatibility_call -from .exceptions import EmptyCommand from . import logs, shells +from .conf import settings, DEFAULT_PRIORITY, ALL_ENABLED +from .exceptions import EmptyCommand +from .utils import compatibility_call +from imp import load_source +from psutil import Process, TimeoutExpired +from subprocess import Popen, PIPE +import os +import six +import sys class Command(object): @@ -21,10 +21,23 @@ class Command(object): :type stderr: basestring """ - self.script = script + self._script = script self.stdout = stdout self.stderr = stderr + try: + self._split_script = shells.split_command(script) + except: + self._split_script = None + + @property + def script(self): + return self._script + + @property + def split_script(self): + return self._split_script + def __eq__(self, other): if isinstance(other, Command): return (self.script, self.stdout, self.stderr) \ diff --git a/thefuck/utils.py b/thefuck/utils.py index 51b9a113..bdcc25b8 100644 --- a/thefuck/utils.py +++ b/thefuck/utils.py @@ -12,16 +12,10 @@ from inspect import getargspec from pathlib import Path import pkg_resources -import six from .conf import settings DEVNULL = open(os.devnull, 'w') -if six.PY2: - from pipes import quote -else: - from shlex import quote - def memoize(fn): """Caches previous calls to the function.""" @@ -144,19 +138,23 @@ def replace_command(command, broken, matched): @memoize -def is_app(command, *app_names): +def is_app(command, *app_names, **kwargs): """Returns `True` if command is call to one of passed app names.""" - for name in app_names: - if command.script == name \ - or command.script.startswith(u'{} '.format(name)): - return True + + at_least = kwargs.pop('at_least', 0) + if kwargs: + raise TypeError("got an unexpected keyword argument '{}'".format(kwargs.keys())) + + if command.split_script is not None and len(command.split_script) > at_least: + return command.split_script[0] in app_names + return False -def for_app(*app_names): +def for_app(*app_names, **kwargs): """Specifies that matching script is for on of app names.""" def _for_app(fn, command): - if is_app(command, *app_names): + if is_app(command, *app_names, **kwargs): return fn(command) else: return False