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

Do not overwrite HISTFILE #64

Open
a-dekker opened this issue Sep 19, 2019 · 5 comments · Fixed by #81
Open

Do not overwrite HISTFILE #64

a-dekker opened this issue Sep 19, 2019 · 5 comments · Fixed by #81
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@a-dekker
Copy link

a-dekker commented Sep 19, 2019

I am using a custom HISTFILE name. And to keep it that way, I also make this variable readonly.

As mcfly does
export HISTFILE="${HISTFILE:-$HOME/.bash_history}"

this leads to an annoying warning:
bash: HISTFILE: readonly variable
A check like

if [ -z "${HISTFILE}" ]; then
    export HISTFILE="${HOME}/.bash_history"
fi

would prevent this warning for me.

@cantino
Copy link
Owner

cantino commented Sep 19, 2019

Thanks @a-dekker, that's a good addition.

@cantino cantino self-assigned this Sep 19, 2019
@cantino cantino added bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed labels May 24, 2020
@cantino
Copy link
Owner

cantino commented May 25, 2020

I'll include a fix for this in #81.

@cantino cantino mentioned this issue May 25, 2020
Merged
@a-dekker
Copy link
Author

It doesn't seem to be fixed, it still contains the line
export HISTFILE="${HISTFILE:-$HOME/.bash_history}"
at https://github.com/cantino/mcfly/blob/master/mcfly.bash#L13

@cantino
Copy link
Owner

cantino commented Jun 28, 2020

Hey @a-dekker, sorry about that. I actually reverted the change, as it was giving me odd issues in BASH where the variable didn't seem to be set.

@cantino cantino reopened this Jun 28, 2020
@a-dekker
Copy link
Author

a-dekker commented Jun 28, 2020

That seems strange, never experienced any issues with the alternative implementation.
The change is also very straightforward: test if the value is set, if not, set the default.
It should do the same as your implementation, except not overwrite it by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants