tf 3 #18
tf 3 #18
Conversation
WalkthroughThe Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
blar-app
bot
added
size S
SummaryThis PR updates the Fish shell integration in the 'thefuck' utility by modifying the 'info' method to fetch the version using 'fish -c echo $FISH_VERSION' instead of the previous command. Additionally, the test for this method has been adjusted to match the new output format, mocking the subprocess to return just the version number. These changes enhance the accuracy and reliability of detecting the Fish shell version within the utility, ensuring consistent behavior across different Fish versions and environments. Pull Request Impact: 0 🔍 See your Change Graph🔄 File Changes Overview
📜 Blar InstructionsBlar Commands
Tags Explanation
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
thefuck_3/tests/shells/test_fish.py(1 hunks)thefuck_3/thefuck/shells/fish.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
thefuck_3/thefuck/shells/fish.py (1)
thefuck_3/tests/shells/test_fish.py (1)
Popen(14-21)
🔇 Additional comments (1)
thefuck_3/tests/shells/test_fish.py (1)
117-117: Test correctly updated to match implementation changes.The mocked output change from
b'fish, version 3.5.9\n'tob'3.5.9'properly aligns with the newfish -c 'echo $FISH_VERSION'command that outputs just the version number.
| proc = Popen(['fish', '-c', 'echo $FISH_VERSION'], | ||
| stdout=PIPE, stderr=DEVNULL) | ||
| version = proc.stdout.read().decode('utf-8').split()[-1] | ||
| version = proc.stdout.read().decode('utf-8').strip() | ||
| return u'Fish Shell {}'.format(version) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM! Cleaner approach to version retrieval.
The change from fish --version to fish -c 'echo $FISH_VERSION' is more direct and eliminates the need for output parsing. This should be more reliable and efficient.
Please verify that the $FISH_VERSION environment variable is available across all supported Fish shell versions:
🏁 Script executed:
#!/bin/bash
# Verify Fish shell version variable availability
echo "Testing Fish shell version retrieval methods"
echo "Method 1 (new): fish -c 'echo \$FISH_VERSION'"
fish -c 'echo $FISH_VERSION' 2>/dev/null || echo "Failed"
echo "Method 2 (old): fish --version"
fish --version 2>/dev/null || echo "Failed"Length of output: 364
Ensure reliable fish version retrieval with --no-config and fallback
Using fish -c 'echo $FISH_VERSION' can load users’ config.fish (producing unexpected output) and may not set $FISH_VERSION in non-interactive or older shells. Please adjust the implementation to:
- Invoke fish with
--no-configto skip~/.config/fish/config.fish. - Fall back to the legacy
--versionflag when$FISH_VERSIONis empty.
Suggested diff:
--- a/thefuck_3/thefuck/shells/fish.py
+++ b/thefuck_3/thefuck/shells/fish.py
@@ -108,7 +108,13 @@ class Fish(Shell):
- proc = Popen(['fish', '-c', 'echo $FISH_VERSION'],
- stdout=PIPE, stderr=DEVNULL)
+ # Skip user config to avoid loading any custom prompts or scripts
+ proc = Popen(
+ ['fish', '--no-config', '-c', 'echo $FISH_VERSION'],
+ stdout=PIPE, stderr=DEVNULL
+ )
+ version = proc.stdout.read().decode('utf-8').strip()
+ # Fallback if FISH_VERSION is empty or unsupported
+ if not version:
+ proc = Popen(['fish', '--no-config', '--version'], stdout=PIPE, stderr=DEVNULL)
+ version = proc.stdout.read().decode('utf-8').strip()
+ return u'Fish Shell {}'.format(version)This ensures consistency across environments and provides a safe fallback.
📝 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.
| proc = Popen(['fish', '-c', 'echo $FISH_VERSION'], | |
| stdout=PIPE, stderr=DEVNULL) | |
| version = proc.stdout.read().decode('utf-8').split()[-1] | |
| version = proc.stdout.read().decode('utf-8').strip() | |
| return u'Fish Shell {}'.format(version) | |
| # Skip user config to avoid loading any custom prompts or scripts | |
| proc = Popen( | |
| ['fish', '--no-config', '-c', 'echo $FISH_VERSION'], | |
| stdout=PIPE, stderr=DEVNULL | |
| ) | |
| version = proc.stdout.read().decode('utf-8').strip() | |
| # Fallback if FISH_VERSION is empty or unsupported | |
| if not version: | |
| proc = Popen( | |
| ['fish', '--no-config', '--version'], | |
| stdout=PIPE, stderr=DEVNULL | |
| ) | |
| version = proc.stdout.read().decode('utf-8').strip() | |
| return u'Fish Shell {}'.format(version) |
🤖 Prompt for AI Agents
In thefuck_3/thefuck/shells/fish.py around lines 108 to 111, update the fish
version retrieval to invoke fish with the --no-config flag to avoid loading user
config files that may produce unexpected output. After reading $FISH_VERSION,
check if it is empty; if so, fall back to running fish --version to get the
version string. This ensures consistent and reliable version detection across
different fish shell versions and environments.
|
✅ No debugger issues found 🐛 Review's done! 🚀 Check out the feedback and let me know if you need anything! – Blar |
|
CODERABBIT |
Summary by CodeRabbit