Merge pull request #36 from jjmason/improve-no-command

Improve no_command

tests/rules/test_no_command.py

@@ -9,16 +9,27 @@ from thefuck.main import Command
 def command_found():
     return b'''No command 'aptget' found, did you mean:
  Command 'apt-get' from package 'apt' (main)
+ Command 'not-installed' from package 'derp' (main)
+ Command 'not-really-used' from package 'whatever' (main)
 aptget: command not found
 '''
 
+@pytest.fixture
+def uninstalled_command_found():
+    return b'''No command 'pish' found, did you mean:
+ Command 'vish' from package 'vish' (universe)
+ Command 'wish' from package 'tk' (main)
+ Command 'fish' from package 'fish' (universe)
+ Command 'pdsh' from package 'pdsh' (universe)
+pish: command not found
+'''
+
 @pytest.fixture
 def command_not_found():
     return b'''No command 'vom' found, but there are 19 similar ones
 vom: command not found
 '''
 
-
 @pytest.fixture
 def bins_exists(request):
     p = patch('thefuck.rules.no_command.which',
@@ -26,6 +37,26 @@ def bins_exists(request):
     p.start()
     request.addfinalizer(p.stop)
 
+@pytest.fixture
+def bin_might_exist(request):
+    def side_effect(name):
+        return name in ['not-really-used', 'apt-get', '/usr/lib/command-not-found', 'test']
+    p = patch('thefuck.rules.no_command.which',
+              side_effect = side_effect)
+    p.start()
+    request.addfinalizer(p.stop)
+            
+
+@pytest.fixture
+def patch_history(request):
+    def side_effect(name):
+        return 2 if name == 'not-really-used' else 12
+    p = patch('thefuck.rules.no_command._count_history_uses',
+              side_effect = side_effect)
+    p.start()
+    request.addfinalizer(p.stop)
+    
+
 
 @pytest.fixture
 def settings():
@@ -34,12 +65,12 @@ def settings():
     return _Settings
 
 
-@pytest.mark.usefixtures('bins_exists')
+@pytest.mark.usefixtures('bin_might_exist', 'patch_history')
-def test_match(command_found, command_not_found, settings):
+def test_match(command_found, command_not_found, uninstalled_command_found, settings):
     with patch('thefuck.rules.no_command.Popen') as Popen:
         Popen.return_value.stderr.read.return_value = command_found
         assert match(Command('aptget install vim', '', ''), settings)
-        Popen.assert_called_once_with('/usr/lib/command-not-found aptget',
+        Popen.assert_called_with('/usr/lib/command-not-found aptget',
                                       shell=True, stderr=PIPE)
         Popen.return_value.stderr.read.return_value = command_not_found
         assert not match(Command('ls', '', ''), settings)
@@ -48,11 +79,14 @@ def test_match(command_found, command_not_found, settings):
         Popen.return_value.stderr.read.return_value = command_found
         assert match(Command('sudo aptget install vim', '', ''),
                      Mock(command_not_found='test'))
-        Popen.assert_called_once_with('test aptget',
+        Popen.assert_called_with('test aptget',
                                       shell=True, stderr=PIPE)
 
+    with patch('thefuck.rules.no_command.Popen') as Popen:
+        Popen.return_value.stderr.read.return_value = uninstalled_command_found
+        assert not match(Command('pish bla blah', '', ''), settings)
 
-@pytest.mark.usefixtures('bins_exists')
+@pytest.mark.usefixtures('bin_might_exist', 'patch_history')
 def test_get_new_command(command_found):
     with patch('thefuck.rules.no_command._get_output',
                return_value=command_found.decode()):

thefuck/rules/no_command.py

@@ -2,7 +2,6 @@ from subprocess import Popen, PIPE
 import re
 from thefuck.utils import which, wrap_settings
 
-
 local_settings = {'command_not_found': '/usr/lib/command-not-found'}
 
 
@@ -12,12 +11,28 @@ def _get_output(command, settings):
     result = Popen(check_script, shell=True, stderr=PIPE)
     return result.stderr.read().decode('utf-8')
 
+def _count_history_uses(name):
+    script = "history | egrep '\\b{}\\b' | wc -l".format(name)
+    result = Popen(script, shell=True,
+                   stdout=PIPE)
+    return int(result.stdout.read())
+
+def _get_candidate_commands(command, settings):
+    output = _get_output(command, settings)
+    if "No command" in output and "from package" in output:
+        fixed_names = re.findall(r"Command '([^']*)' from package",
+                            output)
+        return [name for name in fixed_names if which(name)]
+    return []
+        
+
 
 @wrap_settings(local_settings)
 def match(command, settings):
     if which(settings.command_not_found):
         output = _get_output(command, settings)
-        return "No command" in output and "from package" in output
+        return len(_get_candidate_commands(command, settings)) != 0
+
 
 
 @wrap_settings(local_settings)
@@ -25,6 +40,7 @@ def get_new_command(command, settings):
     output = _get_output(command, settings)
     broken_name = re.findall(r"No command '([^']*)' found",
                              output)[0]
-    fixed_name = re.findall(r"Command '([^']*)' from package",
+    candidates = _get_candidate_commands(command, settings)
-                            output)[0]
+    fixed_name = sorted(candidates, key=_count_history_uses, reverse=True)[0]
-    return command.script.replace(broken_name, fixed_name, 1)
+    return command.script.replace(broken_name, fixed_name)
+