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

BUG: McFly exporting HISTFILE can cause conflicts between shells #249

Open
dithpri opened this issue Apr 22, 2022 · 6 comments
Open

BUG: McFly exporting HISTFILE can cause conflicts between shells #249

dithpri opened this issue Apr 22, 2022 · 6 comments

Comments

@dithpri
Copy link

dithpri commented Apr 22, 2022

I pretty much exclusively use zsh. However, I sometimes need to run bash (or run it from nix-shell).
I've had no problems with this until I installed McFly.

Since McFly exports HISTFILE when loaded in an interactive zsh shell:
https://github.com/cantino/mcfly/blob/75808a60c78f65e90767ee53c9468086fcdf0cc5/mcfly.zsh#L13=

If bash is run from such a shell, because HISTFILE is exported, bash's HISTFILE is the same as zsh's. As it happens, the default history file size for bash is 500. So, every time I run bash, my zsh history file gets truncated:

$ wc -l "$HISTFILE"
7475 /home/dith/.local/share/zsh/history
$ bash -i =(echo true)
$ wc -l "$HISTFILE"
501 /home/dith/.local/share/zsh/history

Moreover, bash saves its history to the zsh HISTFILE...

The workaround is to set a different HISTFILE in .bashrc, but this is most definitely a bug introduced by McFly exporting the HISTFILE variable.
McFly should honour the variable not being exported and define its own variables for internal use instead of exporting ones that don't belong to it.

Suggestion: export MCFLY_HISTFILE="${HISTFILE:-$HOME/.zsh_history}" (as it seems to be done for fish)

Same goes for the bash script https://github.com/cantino/mcfly/blob/75808a60c78f65e90767ee53c9468086fcdf0cc5/mcfly.bash#L13=

@cantino
Copy link
Owner

cantino commented May 8, 2022

If you change mcfly.bash to something like the following, does everything start working for you?

# Ensure MCFLY_HISTFILE exists.
default_histfile="${HISTFILE:-$HOME/.bash_history}"
export MCFLY_HISTFILE="${MCFLY_HISTFILE:-$default_histfile}"
if [[ ! -r "${MCFLY_HISTFILE}" ]]; then
  echo "McFly: ${MCFLY_HISTFILE} does not exist or is not readable. Please fix this or set MCFLY_HISTFILE to something else before using McFly."
  return 1
fi

@dithpri
Copy link
Author

dithpri commented May 13, 2022

No, that doesn't work, because HISTFILE is exported in mcfly.zsh (the issue I'm having is running bash from a zsh shell).

But changing mcfly.zsh to a similar snippet:

# Ensure MCFLY_HISTFILE exists.
default_histfile="${HISTFILE:-$HOME/.zsh_history}"
export MCFLY_HISTFILE="${MCFLY_HISTFILE:-$default_histfile}"
if [[ ! -r "${MCFLY_HISTFILE}" ]]; then
  echo "McFly: ${MCFLY_HISTFILE} does not exist or is not readable. Please fix this or set MCFLY_HISTFILE to something else before using McFly."
  return 1
fi

does fix this for me.

I think both your suggestion and this one will have to be applied so that it works both ways (so that zsh and bash don't share the same HISTFILE unless set so by the user in both shells' rcs.

This however does not update MCFLY_HISTFILE (but that worked the same way before) when a sub-shell is run, but that's a separate, related, bug.

@cantino
Copy link
Owner

cantino commented May 15, 2022

That makes sense. We should just make this change everywhere. Would you be up for making it, or should I?

@dithpri
Copy link
Author

dithpri commented May 19, 2022

I'll submit a pull request sometime this week.

@AndrewKvalheim
Copy link

I tried McFly for about five minutes today and this led to the silent deletion of years of my shell history.

@cantino
Copy link
Owner

cantino commented Dec 8, 2022

I'm sorry to hear that Andrew, we'll prioritize getting this fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants