scorphus
commented
2 years ago Seems to me that this is the history rule yielding different results for different shells, as they have different histories.
Open septatrix opened 2 years ago
scorphus
commented
2 years ago Seems to me that this is the history rule yielding different results for different shells, as they have different histories.
septatrix
commented
2 years ago What history rule are you referring to? Is it possible to suppress some rules to check if your suspicion is correct?
scorphus
commented
2 years ago I mean the history rule that "tries to replace command with the most similar command from history". Check the How it works section of the README.
Yes, that's possible. You can also check Settings section of the README for that. This is what you want:
THEFUCK_RULES=history thefuck "echoo ok"But, thanks to you I just saw that The Fuck still uses the outdated location of the history file:
Which must be fixed.
Thanks for filing this issue!
mainrs
commented
2 years ago Thanks for linking me to the issue. It might be that the outdated history file location is the culprit. When I use thefuck it doesn't update my history file. It always shows fuck --yeah.
mainrs
commented
2 years ago @scorphus Do you want to keep backwards compatibility with older fish shells? Or can I simply update the history file location?
septatrix
commented
2 years ago Yes, that's possible. You can also check Settings section of the README for that. This is what you want:
THEFUCK_RULES=history thefuck "echoo ok"
In that case I get No fucks given.
With THEFUCK_EXCLUDE_RULES=history I get multiple suggestions (Corrected commands: CorrectedCommand(script=/usr/bin/nano /bin/sh +1; and echoo ok, side_effect=None, priority=1000), CorrectedCommand(script=echo ok, side_effect=None, priority=3000), CorrectedCommand(script=echo o ok, side_effect=None, priority=4000), CorrectedCommand(script=choom ok, side_effect=None, priority=6000), CorrectedCommand(script=chroot ok, side_effect=None, priority=9000).
When using the my reproduction example with git push I still get fish -ic "git push" when excluding history and No fucks given when only including it. However when excluding long_form_help I get No fucks given instead of the fish -ic "git push"
septatrix
commented
2 years ago Oh I think I found the culprit. git is an alias/wrapper to hub on my machine:
$ type git
git is a function with definition
# Defined via `source`
function git --wraps=hub --description 'Alias for hub, which wraps git to provide extra functionality with GitHub.'
hub $argv
end@scorphus Do you want to keep backwards compatibility with older fish shells? Or can I simply update the history file location?
@mainrs: I thought about that. Then I went checking when that was changed and it's been almost 6 years ago. So I'd say there's no need for that. We might want to support the XDG Base Directory Specification though (XDG_DATA_HOME in this case). In a way that another hardcoded path won't still be compatible.
[...] In that case I get
No fucks given.
@septatrix: so it really seems to be the absence of history when using fish.
[...] When using the my reproduction example with
git pushI still getfish -ic "git push"when excludinghistoryandNo fucks givenwhen only including it. However when excludinglong_form_helpI getNo fucks giveninstead of thefish -ic "git push"
What does type git output?
scorphus
commented
2 years ago Oh I think I found the culprit.
gitis an alias/wrapper tohubon my machine:$ type git git is a function with definition # Defined via `source` function git --wraps=hub --description 'Alias for hub, which wraps git to provide extra functionality with GitHub.' hub $argv end
Oh! There we have it! You'll want to set "overridden aliases" in that case.
scorphus
commented
2 years ago Please check this out: https://github.com/nvbn/thefuck/wiki/Shell-aliases#fish
septatrix
commented
2 years ago Yes that solves it, thanks. Out of curiosity why is this necessary? All the other shells do not seem to require this and my first impression was that this is required to execute aliases though to my knowledge eval does not need this...
scorphus
commented
2 years ago Please, have a look into 891fbe7 for the motivation.
One other thing is that I now see that Fish's alias is not so slow as it used to be and might be a better option than parsing the output and caching in The Fuck. More so considering cases where aliases are declared outside of config.fish.
How fast is it for you? i.e.
time set foo (alias)Please, have a look into 891fbe7 for the motivation.
Thanks but I actually meant the wrapping with fish -ic "..." which apparently got introduced way back in 9debcdf676a212cc41645acaf4253b7e6e580974. This sets fish apart from other shells and I do not know why. Some comment which got moved around in that commit said that fish does not support aliases but that is from ancient times should that have been the case.
Simply removing that whole logic seems to work fine and I could not detect any wrong behaviour during my short testing:
def get_aliases(self):
overridden = set() # these do not have to be provided anymore
functions = _get_functions(overridden)
raw_aliases = _get_aliases(overridden)
functions.update(raw_aliases)
return functions
def _expand_aliases(self, command_script):
aliases = self.get_aliases()
binary = command_script.split(' ')[0]
if binary in aliases:
return command_script.replace(binary, aliases[binary], 1)
# note the missing elif
return command_scriptI think I will go with that until I run into some errors at which point I can still switch to the THEFUCK_OVERRIDDEN_ALIASES solution.
One other thing is that I now see that Fish's
aliasis not so slow as it used to be and might be a better option than parsing the output and caching in The Fuck. More so considering cases where aliases are declared outside ofconfig.fish.How fast is it for you? i.e.
time set foo (alias)
~ $ time set foo (alias)
________________________________________________________
Executed in 9.00 micros fish external
usr time 9.00 micros 9.00 micros 0.00 micros
sys time 2.00 micros 2.00 micros 0.00 microsI think I will go with that until I run into some errors at which point I can still switch to the
THEFUCK_OVERRIDDEN_ALIASESsolution.
It might as well work for you.
But that's needed when the command being fixed is in fact a function in Fish. Think of how pyenv works in Fish, or npm, or rbenv. These work as native fish functions that rely on bash scripts. And these scripts are only available in your running shell, unless you configure them to be available system wide — which we often don't do, we use plugins and care none about that.
septatrix
commented
2 years ago Oh I see. Yeah that does indeed seem somewhat tricky... And the other shells simply have nothing comparable to fish -ic or why do they not have something similar?
scorphus
commented
2 years ago They don't need that because they can speak the same language (or mostly the same). Whereas Fish has a different language.
septatrix
commented
2 years ago If I understand correctly you mean functions like python venv's deactivate? Those also do not work with bash and thefuck:
[septatrix@thinkpad-e570 ~]$ function asdf() { echo asdf; }
[septatrix@thinkpad-e570 ~]$ asdf
asdf
[septatrix@thinkpad-e570 ~]$ THEFUCK_DEBUG=true fuck
DEBUG: Run with settings: {'alter_history': True,
'debug': True,
'env': {'GIT_TRACE': '1', 'LANG': 'C', 'LC_ALL': 'C'},
'exclude_rules': [],
'excluded_search_path_prefixes': [],
'history_limit': None,
'instant_mode': False,
'no_colors': False,
'num_close_matches': 3,
'priority': {},
'repeat': False,
'require_confirmation': True,
'rules': [<const: All rules enabled>],
'slow_commands': ['lein', 'react-native', 'gradle', './gradlew', 'vagrant'],
'user_dir': PosixPath('/home/septatrix/.config/thefuck'),
'wait_command': 3,
'wait_slow_command': 15}
DEBUG: Received output: /bin/sh: line 1: asdf: command not foundMore in the way of the pyenv command in Bash, that turns out to be a function in Fish:
function pyenv
set cmd $argv[1]
set -e argv[1]
switch "$cmd"
case activate deactivate rehash shell virtualenvwrapper virtualenvwrapper_lazy
command pyenv "sh-$cmd" $argv | source
case '*'
command pyenv "$cmd" $argv
end
endSo I guess the fact that Fish is much more function-oriented than Bash — in a way that fish users create quite a lot of functions (rather than scripts for bash users) and many things are built with functions — required The Fuck to be aware of functions declared in Fish. And that's why the -i part is required in fish -ic "...", as many of those functions are only loaded during an interactive session of the shell (so non-interactive scripts can be fast, but I digress.)
septatrix
commented
2 years ago Though would that not also apply to other shells like bash or zsh when using oh-my-zsh or similar? They too provide a -i flag to load functions and aliases.
Also this feels more of a step which should be done in to_shell (is this even used?) or at least the expanded command should only be used for get_output and not set as the value for Command.script because to me it seems like the wrapping with fish -ic just confuses every rule which exists.
While at it why not just unconditionally wrap the command with fish -ic instead of spawning fish -ic {functions,aliases} subprocesses just to figure out which functions/aliases exists. Spawning a single interactive session should be slower than spawning two with a potential third.
Sorry if I just overlook some obvious flaw in my reasoning but I really like the fuck and to me it seems like that is just some legacy code which might have been necessary once but probably is not needed anymore.
The output of
thefuck --version(something likeThe Fuck 3.1 using Python 3.5.0 and Bash 4.4.12(1)-release):Your system (Debian 7, ArchLinux, Windows, etc.):
How to reproduce the bug
git commit --amendand changing the commit message.--force.fuck.The output of The Fuck with
THEFUCK_DEBUG=trueexported (typically executeexport THEFUCK_DEBUG=truein your shell before The Fuck):Log
``` fuck DEBUG: Run with settings: {'alter_history': True, 'debug': True, 'env': {'GIT_TRACE': '1', 'LANG': 'C', 'LC_ALL': 'C'}, 'exclude_rules': [], 'excluded_search_path_prefixes': [], 'history_limit': None, 'instant_mode': False, 'no_colors': False, 'num_close_matches': 3, 'priority': {}, 'repeat': False, 'require_confirmation': True, 'rules': [If the bug only appears with a specific application, the output of that application and its version:
Anything else you think is relevant: