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

omz_history -p doesn't work as expected #12334

Closed
MoAlkhateeb opened this issue Apr 8, 2024 · 5 comments · Fixed by #12338
Closed

omz_history -p doesn't work as expected #12334

MoAlkhateeb opened this issue Apr 8, 2024 · 5 comments · Fixed by #12338
Assignees
Labels
Bug Something isn't working

Comments

@MoAlkhateeb
Copy link
Contributor

Describe the bug

  • In bash, in order to not save the history for the current session you use history -c. To do the same in zsh you use history -p instead.

  • Using ohmyzsh, history -c clears the history file completely instead of just not saving the history for the current session.

  • The history file path is stored in the HISTFILE environment variable. By default, it's the $HOME/.zsh_history file.

  • When using oh-my-zsh, history is aliased to the omz_history function. which is defined as follows:

    ohmyzsh/lib/history.zsh

    Lines 2 to 18 in bf713e2

    function omz_history {
    local clear list
    zparseopts -E c=clear l=list
    if [[ -n "$clear" ]]; then
    # if -c provided, clobber the history file
    echo -n >| "$HISTFILE"
    fc -p "$HISTFILE"
    echo >&2 History file deleted.
    elif [[ -n "$list" ]]; then
    # if -l provided, run as if calling `fc' directly
    builtin fc "$@"
    else
    # unless a number is provided, show all history events (starting from 1)
    [[ ${@[-1]-} = *[0-9]* ]] && builtin fc -l "$@" || builtin fc -l "$@" 1
    fi
    }

  • When attempting to clear the history completely, you run history -c or its equivalent omz_history -c. This clears the history file completely as intended.

  • When attempting to not save history for the current session but not tampering with the history file, you would run history -p.

  • However, when attempting to run history -p with ohmyzsh this results in running builtin fc -l "$@" 1. Here "$@" is set to -p which results in builtin fc -l -p 1. This indeed does not save the current session history to the original history file. However:

  • What this causes is that the HISTFILE environment variable is overwritten with the value 1. A file named 1 is automatically created in the current working directory and the session history is stored in it. Any attempt to remove the file within the session will result in it being recreated whenever any interactive command is executed until the session is exited.

  • When the session is exited, the file still exists.

References:

Steps to reproduce

  1. Start a new session
  2. Navigate to a directory with no files called 1.
  3. echo $HISTFILE should result in $HOME/.zsh_history by default.
  4. run history -p
  5. echo $HISTFILE results in 1.
  6. ls you find a file called 1.

Expected behavior

  1. history -p unsets the HISTFILE environment variable.
  2. No file is created.
  3. Current session history is not preserved beyond the session.

Screenshots and recordings

asciicast

OS / Linux distribution

macOS Sonoma 14.4.1

Zsh version

5.9

Oh My Zsh version

master (bf713e2)

Terminal emulator

iTerm2

If using WSL on Windows, which version of WSL

None

Additional context

No response

@MoAlkhateeb MoAlkhateeb added the Bug Something isn't working label Apr 8, 2024
@carlosala
Copy link
Member

I absolutely agree. @mcornella what do you think about? I'd remove

[[ ${@[-1]-} = *[0-9]* ]] && builtin fc -l "$@" || builtin fc -l "$@" 1
and directly call fc -l "$@" instead of printing the whole history.

@carlosala carlosala self-assigned this Apr 8, 2024
@MoAlkhateeb
Copy link
Contributor Author

I came up with the same conclusion, replacing the else block with builtin fc -l "$@". That way the user can have some control over the underlying fc call and at the same time users executing just history are not left out.

Should I submit a PR for this? I would love to.

@carlosala
Copy link
Member

Sure! Let's discuss in the PR with @mcornella

@MoAlkhateeb
Copy link
Contributor Author

Great! #12338

@mcornella
Copy link
Member

Thanks @MoAlkhateeb!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants