#N/A: Improve how version is fetched for all shells (#920)

tests/shells/test_bash.py

@@ -11,6 +11,11 @@ class TestBash(object):
     def shell(self):
         return Bash()
 
+    @pytest.fixture(autouse=True)
+    def Popen(self, mocker):
+        mock = mocker.patch('thefuck.shells.bash.Popen')
+        return mock
+
     @pytest.fixture(autouse=True)
     def shell_aliases(self):
         os.environ['TF_SHELL_ALIASES'] = (
@@ -74,7 +79,12 @@ class TestBash(object):
         config_exists.return_value = False
         assert not shell.how_to_configure().can_configure_automatically
 
-    def test_info(self, shell, mocker):
-        patch = mocker.patch('thefuck.shells.bash.Popen')
-        patch.return_value.stdout.read.side_effect = [b'3.5.9']
+    def test_info(self, shell, Popen):
+        Popen.return_value.stdout.read.side_effect = [b'3.5.9']
         assert shell.info() == 'Bash 3.5.9'
+
+    def test_get_version_error(self, shell, Popen):
+        Popen.return_value.stdout.read.side_effect = OSError
+        with pytest.raises(OSError):
+            shell._get_version()
+        assert Popen.call_args[0][0] == ['bash', '-c', 'echo $BASH_VERSION']

tests/shells/test_fish.py

@@ -116,7 +116,17 @@ class TestFish(object):
         config_exists.return_value = False
         assert not shell.how_to_configure().can_configure_automatically
 
-    def test_info(self, shell, Popen):
+    def test_get_version(self, shell, Popen):
         Popen.return_value.stdout.read.side_effect = [b'fish, version 3.5.9\n']
-        assert shell.info() == 'Fish Shell 3.5.9'
+        assert shell._get_version() == '3.5.9'
+        assert Popen.call_args[0][0] == ['fish', '--version']
+
+    @pytest.mark.parametrize('side_effect, exception', [
+        ([b'\n'], IndexError),
+        (OSError('file not found'), OSError),
+    ])
+    def test_get_version_error(self, side_effect, exception, shell, Popen):
+        Popen.return_value.stdout.read.side_effect = side_effect
+        with pytest.raises(exception):
+            shell._get_version()
         assert Popen.call_args[0][0] == ['fish', '--version']

tests/shells/test_generic.py

@@ -43,3 +43,14 @@ class TestGeneric(object):
 
     def test_how_to_configure(self, shell):
         assert shell.how_to_configure() is None
+
+    @pytest.mark.parametrize('side_effect, expected_info, warn', [
+        ([u'3.5.9'], u'Generic Shell 3.5.9', False),
+        ([OSError], u'Generic Shell', True),
+    ])
+    def test_info(self, side_effect, expected_info, warn, shell, mocker):
+        warn_mock = mocker.patch('thefuck.shells.generic.warn')
+        shell._get_version = mocker.Mock(side_effect=side_effect)
+        assert shell.info() == expected_info
+        assert warn_mock.called is warn
+        assert shell._get_version.called

tests/shells/test_powershell.py

@@ -10,6 +10,11 @@ class TestPowershell(object):
     def shell(self):
         return Powershell()
 
+    @pytest.fixture(autouse=True)
+    def Popen(self, mocker):
+        mock = mocker.patch('thefuck.shells.powershell.Popen')
+        return mock
+
     def test_and_(self, shell):
         assert shell.and_('ls', 'cd') == '(ls) -and (cd)'
 
@@ -20,3 +25,20 @@ class TestPowershell(object):
 
     def test_how_to_configure(self, shell):
         assert not shell.how_to_configure().can_configure_automatically
+
+    @pytest.mark.parametrize('side_effect, expected_version, call_args', [
+        ([b'''Major  Minor  Build  Revision
+-----  -----  -----  --------
+5      1      17763  316     \n'''], 'PowerShell 5.1.17763.316', ['powershell.exe']),
+        ([IOError, b'PowerShell 6.1.2\n'], 'PowerShell 6.1.2', ['powershell.exe', 'pwsh'])])
+    def test_info(self, side_effect, expected_version, call_args, shell, Popen):
+        Popen.return_value.stdout.read.side_effect = side_effect
+        assert shell.info() == expected_version
+        assert Popen.call_count == len(call_args)
+        assert all([Popen.call_args_list[i][0][0][0] == call_arg for i, call_arg in enumerate(call_args)])
+
+    def test_get_version_error(self, shell, Popen):
+        Popen.return_value.stdout.read.side_effect = RuntimeError
+        with pytest.raises(RuntimeError):
+            shell._get_version()
+        assert Popen.call_args[0][0] == ['powershell.exe', '$PSVersionTable.PSVersion']

tests/shells/test_tcsh.py

@@ -61,3 +61,17 @@ class TestTcsh(object):
                                                     config_exists):
         config_exists.return_value = False
         assert not shell.how_to_configure().can_configure_automatically
+
+    def test_info(self, shell, Popen):
+        Popen.return_value.stdout.read.side_effect = [
+            b'tcsh 6.20.00 (Astron) 2016-11-24 (unknown-unknown-bsd44) \n']
+        assert shell.info() == 'Tcsh 6.20.00'
+        assert Popen.call_args[0][0] == ['tcsh', '--version']
+
+    @pytest.mark.parametrize('side_effect, exception', [
+        ([b'\n'], IndexError), (OSError, OSError)])
+    def test_get_version_error(self, side_effect, exception, shell, Popen):
+        Popen.return_value.stdout.read.side_effect = side_effect
+        with pytest.raises(exception):
+            shell._get_version()
+        assert Popen.call_args[0][0] == ['tcsh', '--version']

tests/shells/test_zsh.py

@@ -11,6 +11,11 @@ class TestZsh(object):
     def shell(self):
         return Zsh()
 
+    @pytest.fixture(autouse=True)
+    def Popen(self, mocker):
+        mock = mocker.patch('thefuck.shells.zsh.Popen')
+        return mock
+
     @pytest.fixture(autouse=True)
     def shell_aliases(self):
         os.environ['TF_SHELL_ALIASES'] = (
@@ -69,7 +74,12 @@ class TestZsh(object):
         config_exists.return_value = False
         assert not shell.how_to_configure().can_configure_automatically
 
-    def test_info(self, shell, mocker):
-        patch = mocker.patch('thefuck.shells.zsh.Popen')
-        patch.return_value.stdout.read.side_effect = [b'3.5.9']
+    def test_info(self, shell, Popen):
+        Popen.return_value.stdout.read.side_effect = [b'3.5.9']
         assert shell.info() == 'ZSH 3.5.9'
+
+    def test_get_version_error(self, shell, Popen):
+        Popen.return_value.stdout.read.side_effect = OSError
+        with pytest.raises(OSError):
+            shell._get_version()
+        assert Popen.call_args[0][0] == ['zsh', '-c', 'echo $ZSH_VERSION']

thefuck/shells/bash.py

@@ -9,6 +9,8 @@ from .generic import Generic
 
 
 class Bash(Generic):
+    friendly_name = 'Bash'
+
     def app_alias(self, alias_name):
         # It is VERY important to have the variables declared WITHIN the function
         return '''
@@ -83,9 +85,8 @@ class Bash(Generic):
             path=config,
             reload=u'source {}'.format(config))
 
-    def info(self):
-        """Returns the name and version of the current shell"""
+    def _get_version(self):
+        """Returns the version of the current shell"""
         proc = Popen(['bash', '-c', 'echo $BASH_VERSION'],
                      stdout=PIPE, stderr=DEVNULL)
-        version = proc.stdout.read().decode('utf-8').strip()
-        return u'Bash {}'.format(version)
+        return proc.stdout.read().decode('utf-8').strip()

thefuck/shells/fish.py

@@ -38,6 +38,8 @@ def _get_aliases(overridden):
 
 
 class Fish(Generic):
+    friendly_name = 'Fish Shell'
+
     def _get_overridden_aliases(self):
         overridden = os.environ.get('THEFUCK_OVERRIDDEN_ALIASES',
                                     os.environ.get('TF_OVERRIDDEN_ALIASES', ''))
@@ -104,12 +106,10 @@ class Fish(Generic):
             path='~/.config/fish/config.fish',
             reload='fish')
 
-    def info(self):
-        """Returns the name and version of the current shell"""
-        proc = Popen(['fish', '--version'],
-                     stdout=PIPE, stderr=DEVNULL)
-        version = proc.stdout.read().decode('utf-8').split()[-1]
-        return u'Fish Shell {}'.format(version)
+    def _get_version(self):
+        """Returns the version of the current shell"""
+        proc = Popen(['fish', '--version'], stdout=PIPE, stderr=DEVNULL)
+        return proc.stdout.read().decode('utf-8').split()[-1]
 
     def put_to_history(self, command):
         try:

thefuck/shells/generic.py

@@ -14,6 +14,8 @@ ShellConfiguration = namedtuple('ShellConfiguration', (
 
 
 class Generic(object):
+    friendly_name = 'Generic Shell'
+
     def get_aliases(self):
         return {}
 
@@ -131,9 +133,18 @@ class Generic(object):
                 'type', 'typeset', 'ulimit', 'umask', 'unalias', 'unset',
                 'until', 'wait', 'while']
 
+    def _get_version(self):
+        """Returns the version of the current shell"""
+        return ''
+
     def info(self):
         """Returns the name and version of the current shell"""
-        return 'Generic Shell'
+        try:
+            version = self._get_version()
+        except Exception as e:
+            warn(u'Could not determine shell version: {}'.format(e))
+            version = ''
+        return u'{} {}'.format(self.friendly_name, version).rstrip()
 
     def _create_shell_configuration(self, content, path, reload):
         return ShellConfiguration(

thefuck/shells/powershell.py

@@ -1,7 +1,11 @@
+from subprocess import Popen, PIPE
+from ..utils import DEVNULL
 from .generic import Generic, ShellConfiguration
 
 
 class Powershell(Generic):
+    friendly_name = 'PowerShell'
+
     def app_alias(self, alias_name):
         return 'function ' + alias_name + ' {\n' \
                '    $history = (Get-History -Count 1).CommandLine;\n' \
@@ -24,3 +28,16 @@ class Powershell(Generic):
             path='$profile',
             reload='& $profile',
             can_configure_automatically=False)
+
+    def _get_version(self):
+        """Returns the version of the current shell"""
+        try:
+            proc = Popen(
+                ['powershell.exe', '$PSVersionTable.PSVersion'],
+                stdout=PIPE,
+                stderr=DEVNULL)
+            version = proc.stdout.read().decode('utf-8').rstrip().split('\n')
+            return '.'.join(version[-1].split())
+        except IOError:
+            proc = Popen(['pwsh', '--version'], stdout=PIPE, stderr=DEVNULL)
+            return proc.stdout.read().decode('utf-8').split()[-1]

thefuck/shells/tcsh.py

@@ -6,6 +6,8 @@ from .generic import Generic
 
 
 class Tcsh(Generic):
+    friendly_name = 'Tcsh'
+
     def app_alias(self, alias_name):
         return ("alias {0} 'setenv TF_SHELL tcsh && setenv TF_ALIAS {0} && "
                 "set fucked_cmd=`history -h 2 | head -n 1` && "
@@ -35,3 +37,8 @@ class Tcsh(Generic):
             content=u'eval `thefuck --alias`',
             path='~/.tcshrc',
             reload='tcsh')
+
+    def _get_version(self):
+        """Returns the version of the current shell"""
+        proc = Popen(['tcsh', '--version'], stdout=PIPE, stderr=DEVNULL)
+        return proc.stdout.read().decode('utf-8').split()[1]

thefuck/shells/zsh.py

@@ -10,6 +10,8 @@ from .generic import Generic
 
 
 class Zsh(Generic):
+    friendly_name = 'ZSH'
+
     def app_alias(self, alias_name):
         # It is VERY important to have the variables declared WITHIN the function
         return '''
@@ -87,9 +89,8 @@ class Zsh(Generic):
             path='~/.zshrc',
             reload='source ~/.zshrc')
 
-    def info(self):
-        """Returns the name and version of the current shell"""
+    def _get_version(self):
+        """Returns the version of the current shell"""
         proc = Popen(['zsh', '-c', 'echo $ZSH_VERSION'],
                      stdout=PIPE, stderr=DEVNULL)
-        version = proc.stdout.read().decode('utf-8').strip()
-        return u'ZSH {}'.format(version)
+        return proc.stdout.read().decode('utf-8').strip()