Skip to content
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.

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

Shell alias clobbers some history lines #422

Closed
sehrgut opened this issue Jan 5, 2016 · 13 comments
Closed

Shell alias clobbers some history lines #422

sehrgut opened this issue Jan 5, 2016 · 13 comments
Labels

Comments

@sehrgut
Copy link

sehrgut commented Jan 5, 2016

I was trying this out, and found that large swathes of my history were missing after running "fuck" a single time. This should not modify history except to insert a command it executes..

@sehrgut
Copy link
Author

sehrgut commented Jan 5, 2016

On examination, it appears fuck is simply appending massive numbers of old history lines again, and missing the most recent several.

screen shot 2016-01-05 at 2 28 52 pm

@pjuhasz
Copy link

pjuhasz commented Jan 8, 2016

This does happen to me as well, but apparently only when I'm in a nested shell session.

So simply opening a terminal and issuing 'fuck' won't mess up the history, but running 'bash', then 'fuck' inside it does.

This is relevant because tmux or screen inevitably leads to a situation with nested shells.
The environment variable $SHLVL can be used to detect nesting.

@mcarton
Copy link
Contributor

mcarton commented Jan 8, 2016

I confirm I have the same problem sometimes, but only when The Fuck fails.

@pjuhasz I don't think we could rely on $SHLVL. On my computer it is 1 in a tty and 3 in a GUI (seems to be 3 = 1 for the WM + 1 for the terminal window + 1 for the shell itself all of which counting as a “shell level”).

@pjuhasz
Copy link

pjuhasz commented Jan 8, 2016

I'm not familiar with the internals of The Fuck, so I don't know what kind of magic does it do to the shell history, but from where I stand adding thousands of lines to it is a bug. Being in a nested shell is only a condition that seems to reliably trigger the bug. If it can be fixed, $SHLVL should be irrelevant.

@mcarton
Copy link
Contributor

mcarton commented Jan 8, 2016

I never use a nested shell/tmux/whatever, so either that's not the problem or there are two problems 😅.

@pjuhasz
Copy link

pjuhasz commented Jan 8, 2016

Now that I've looked into the source, I see that the program wants to append to the history by simply and brutally opening the history file on the disk (.bash_history) and writing the line to it.

This is unfortunately not a good strategy: by default, bash does not write the history of the current session to .bash_history, only later when it (the bash process) exits. The situation is further complicated when you use nested shells: the outermost one has the history of its session in its own memory, then when you start the second shell it starts to accumulate its own history in memory. When you exit the inner shell it updates .bash_history, then when you finally exit the terminal/logout etc., the outermost shell saves its own history.

I don't yet see where this fails and how dumping thousands of entries into the history can happen, but writing to .bash_history directly clearly messes up the order of commands even in the simple case of a single shell in a tty. The corrected command should go at the end of the history which is collected in the shell session's memory.

Bash appears to have the built-in command 'history -s' for exactly this purpose.

Alternatively (or orthogonally to this discussion), there should be a configuration option for The Fuck so that it optionally doesn't, ahem, fuck with history at all.

@mcarton
Copy link
Contributor

mcarton commented Jan 8, 2016

zsh seems to have print -s for that. I'll look into that tomorrow.

Alternatively (or orthogonally to this discussion), there should be a configuration option for The Fuck so that it optionally doesn't, ahem, fuck with history at all.

Hey, it's called The Fuck and it fucks your history, what did you expect?
(Ok, I'm out 😅)

@mcarton
Copy link
Contributor

mcarton commented Jan 9, 2016

Here is what I did: https://github.com/mcarton/thefuck/tree/histfile
Unfortunately, bashhistory -s and zshprint -s are not equivalent and this is a breaking change with zsh according to travis.

@sehrgut
Copy link
Author

sehrgut commented Jan 9, 2016

I use bash, not zsh, but from a cursory look at the zsh docs, perhaps the fc builtin could perform the "replace" logic by capturing entries 0 through n-1, writing them back as the new history, and then appending?

nvbn added a commit that referenced this issue Jan 13, 2016
nvbn added a commit that referenced this issue Jan 13, 2016
#422: Add `alter_history` settings option
@nvbn
Copy link
Owner

nvbn commented Jan 17, 2016

As temporary solutin alter_history settings option can be used.

@pjuhasz
Copy link

pjuhasz commented Jan 17, 2016

Thanks.

mcarton's modification above seems to fix the problem, though.

@scorphus
Copy link
Collaborator

Ping, @sehrgut and @pjuhasz. Can we call this fixed and close the issue?

@pjuhasz
Copy link

pjuhasz commented May 19, 2016

Yes, I guess.

amtrivedi91 added a commit to amtrivedi91/thefuck that referenced this issue Aug 31, 2016
amtrivedi91 added a commit to amtrivedi91/thefuck that referenced this issue Aug 31, 2016
amtrivedi91 added a commit to amtrivedi91/thefuck that referenced this issue Aug 31, 2016
amtrivedi91 added a commit to amtrivedi91/thefuck that referenced this issue Aug 31, 2016
@nvbn nvbn added the obsolete label Mar 14, 2017
@nvbn nvbn closed this as completed Mar 14, 2017
riley-martine pushed a commit to riley-martine/thefuck that referenced this issue Dec 7, 2023
riley-martine pushed a commit to riley-martine/thefuck that referenced this issue Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants