Skip to content

tf 4 #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

tf 4 #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions thefuck_4/tests/shells/test_fish.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ def Popen(self, mocker):
mock.return_value.stdout.read.side_effect = [(
b'cd\nfish_config\nfuck\nfunced\nfuncsave\ngrep\nhistory\nll\nls\n'
b'man\nmath\npopd\npushd\nruby'),
(b'alias fish_key_reader /usr/bin/fish_key_reader\nalias g git\n'
b'alias alias_with_equal_sign=echo\ninvalid_alias'), b'func1\nfunc2', b'']
b'alias fish_key_reader /usr/bin/fish_key_reader\nalias g git']
Copy link

@coderabbitai coderabbitai bot May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Restore edge case testing to maintain robust test coverage.

The mocked alias output was simplified to only include valid formats, removing important edge case testing that validates the robustness of the alias parsing logic.

The previous test cases likely included formats like alias_with_equal_sign=echo and invalid_alias to ensure the parser could handle real-world Fish shell output variations. Removing these tests reduces coverage for potential production scenarios.

Consider restoring edge case testing:

 @pytest.fixture(autouse=True)
 def Popen(self, mocker):
     mock = mocker.patch('thefuck.shells.fish.Popen')
     mock.return_value.stdout.read.side_effect = [(
         b'cd\nfish_config\nfuck\nfunced\nfuncsave\ngrep\nhistory\nll\nls\n'
         b'man\nmath\npopd\npushd\nruby'),
-        b'alias fish_key_reader /usr/bin/fish_key_reader\nalias g git']
+        b'alias fish_key_reader /usr/bin/fish_key_reader\nalias g git\n'
+        b'alias_with_equal_sign=echo\ninvalid_alias\n']
     return mock

Then add a separate test to verify that malformed aliases are properly handled without crashing.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
b'alias fish_key_reader /usr/bin/fish_key_reader\nalias g git']
@pytest.fixture(autouse=True)
def Popen(self, mocker):
mock = mocker.patch('thefuck.shells.fish.Popen')
mock.return_value.stdout.read.side_effect = [(
b'cd\nfish_config\nfuck\nfunced\nfuncsave\ngrep\nhistory\nll\nls\n'
b'man\nmath\npopd\npushd\nruby'),
- b'alias fish_key_reader /usr/bin/fish_key_reader\nalias g git']
+ b'alias fish_key_reader /usr/bin/fish_key_reader\nalias g git\n'
+ b'alias_with_equal_sign=echo\ninvalid_alias\n']
return mock
🤖 Prompt for AI Agents 
In thefuck_4/tests/shells/test_fish.py at line 19, the test was simplified by
removing edge case alias formats, which reduces coverage of the alias parsing
logic. Restore the original mocked alias output to include edge cases such as
aliases with equal signs and invalid alias lines. Additionally, add a new test
case specifically to verify that malformed aliases are handled gracefully
without causing crashes.

return mock

@pytest.mark.parametrize('key, value', [
Expand Down Expand Up @@ -70,9 +69,7 @@ def test_get_aliases(self, shell):
'pushd': 'pushd',
'ruby': 'ruby',
'g': 'git',
'fish_key_reader': '/usr/bin/fish_key_reader',
'alias_with_equal_sign': 'echo'}
assert shell.get_aliases() == {'func1': 'func1', 'func2': 'func2'}
'fish_key_reader': '/usr/bin/fish_key_reader'}

def test_app_alias(self, shell):
assert 'function fuck' in shell.app_alias('fuck')
Expand Down
14 changes: 3 additions & 11 deletions thefuck_4/thefuck/shells/fish.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,9 @@ def _get_functions(overridden):
def _get_aliases(overridden):
aliases = {}
proc = Popen(['fish', '-ic', 'alias'], stdout=PIPE, stderr=DEVNULL)
alias_out = proc.stdout.read().decode('utf-8').strip()
if not alias_out:
return aliases
for alias in alias_out.split('\n'):
for separator in (' ', '='):
split_alias = alias.replace('alias ', '', 1).split(separator, 1)
if len(split_alias) == 2:
name, value = split_alias
break
else:
continue
alias_out = proc.stdout.read().decode('utf-8').strip().split('\n')
for alias in alias_out:
name, value = alias.replace('alias ', '', 1).split(' ', 1)
Comment on lines +23 to +25
Copy link

@blar-app blar-app bot May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Error 🐛 Bug

Alias parsing fails on empty output.

Issue Explanation
  • proc.stdout.read().decode('utf-8').strip() returns an empty string when no aliases are defined.
  • Splitting the empty string on '\n' yields [''].
  • Iteration yields alias = ''.
  • alias.replace('alias ', '', 1).split(' ', 1) on '' returns [''].
  • name, value = ... unpack raises ValueError: not enough values to unpack.
  • Occurs in _get_aliases at blarApp/open-benchmark/thefuck_4/thefuck/shells/fish.py:_get_aliases
alias_out = proc.stdout.read().decode('utf-8').strip().split('\n')
for alias in alias_out:
    name, value = alias.replace('alias ', '', 1).split(' ', 1)

Reply if you have any questions or let me know if I missed something.
Don't forget to react with a 👍 or 👎 to the comments made by Blar to help us improve.

Comment on lines +23 to +25
Copy link

@blar-app blar-app bot May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Error 🐛 Bug

Parsing fails for alias definitions without space separators.

Issue Explanation
  • In blarApp/open-benchmark/thefuck_4/thefuck/shells/fish.py::_get_aliases, the code uses alias.replace('alias ', '', 1).split(' ', 1).
  • split(' ', 1) only splits on the first space.
  • Definitions like alias foo=bar contain no space between the name and value.
  • split(' ', 1) returns a single element, causing a ValueError on unpack.
  • There is no fallback to use = as a separator.
name, value = alias.replace('alias ', '', 1).split(' ', 1)

Reply if you have any questions or let me know if I missed something.
Don't forget to react with a 👍 or 👎 to the comments made by Blar to help us improve.

if name not in overridden:
aliases[name] = value
return aliases
Comment on lines +23 to 28
Copy link

@coderabbitai coderabbitai bot May 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for malformed alias lines to prevent runtime crashes.

The simplified parsing logic assumes all alias lines follow the exact "alias name value" format, but lacks error handling for edge cases that could cause a ValueError.

Potential issues:

  • Lines that don't contain a space after removing "alias " will cause unpacking errors
  • Empty lines or malformed output could crash the function
  • Lines not starting with "alias " will have unexpected behavior

Apply this diff to add robust error handling:

 @cache('~/.config/fish/config.fish')
 def _get_aliases(overridden):
     aliases = {}
     proc = Popen(['fish', '-ic', 'alias'], stdout=PIPE, stderr=DEVNULL)
     alias_out = proc.stdout.read().decode('utf-8').strip().split('\n')
     for alias in alias_out:
+        if not alias.strip() or not alias.startswith('alias '):
+            continue
+        try:
             name, value = alias.replace('alias ', '', 1).split(' ', 1)
+        except ValueError:
+            # Skip malformed alias lines
+            continue
         if name not in overridden:
             aliases[name] = value
     return aliases
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
alias_out = proc.stdout.read().decode('utf-8').strip().split('\n')
for alias in alias_out:
name, value = alias.replace('alias ', '', 1).split(' ', 1)
if name not in overridden:
aliases[name] = value
return aliases
@cache('~/.config/fish/config.fish')
def _get_aliases(overridden):
aliases = {}
proc = Popen(['fish', '-ic', 'alias'], stdout=PIPE, stderr=DEVNULL)
alias_out = proc.stdout.read().decode('utf-8').strip().split('\n')
for alias in alias_out:
if not alias.strip() or not alias.startswith('alias '):
continue
try:
name, value = alias.replace('alias ', '', 1).split(' ', 1)
except ValueError:
# Skip malformed alias lines
continue
if name not in overridden:
aliases[name] = value
return aliases
🤖 Prompt for AI Agents 
In thefuck_4/thefuck/shells/fish.py around lines 23 to 28, the alias parsing
code assumes every line starts with "alias " and contains a space to split into
name and value, which can cause runtime errors if the line is malformed. To fix
this, add error handling by first checking if the line starts with "alias ",
then safely splitting it and handling cases where the split does not produce two
parts. Skip or ignore lines that do not meet these conditions to prevent
ValueError and ensure robust parsing.

Expand Down