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

Reverting to old behaviour when running just history #12341

Merged
merged 3 commits into from Apr 9, 2024

Conversation

MoAlkhateeb
Copy link
Contributor

Handles:
#12338 (comment)
#12338 (comment)

Running history prints the whole history starting from 1.

@ohmyzsh ohmyzsh bot added the Area: core Issue or PR related to core parts of the project label Apr 9, 2024
@mcornella mcornella merged commit 605d766 into ohmyzsh:master Apr 9, 2024
2 checks passed
@brettmiller
Copy link

At least in my case it still only shows the last 16 records. It looks like possibly because of the alias that's set when $HIST_STAMPS is set.

@carlosala
Copy link
Member

At least in my case it still only shows the last 16 records. It looks like possibly because of the alias that's set when $HIST_STAMPS is set.

Are you using history directly, or with any parameter?

@brettmiller
Copy link

Directly with no parameters.

$ history | wc -l
16
$ which history
history: aliased to omz_history -i

@MoAlkhateeb
Copy link
Contributor Author

Directly with no parameters.

$ history | wc -l
16
$ which history
history: aliased to omz_history -i

Since history is aliased to omz_history -i and not omz_history, you automatically send a parameter -i. To get the full history, you could run history 1.

I think we should keep the parsing as is because there are many flags (-d, -i, -f, ...) that only change the format of the output of the command. However, another way of handling this would be to explicitly handle the flags that change environment variables (i.e. may have side-effects) differently and make all the other (format) flags print from 1.

Please let me know what your thoughts are.

@brettmiller
Copy link

I'm not setting the history alias to omz_history -i that's coming from lib/history.zsh. So, to me it makes since to explicitly handle the aliases it sets as well. Testing locally adding

  elif [[ $# -eq 1 ]]; then
    if [[ $1 -eq "-f" || $1 -eq "-E" || $1 -eq "-i" ]]; then
      builtin fc -l $1 1
    fi

between the if [[ $# -eq 0 ]]; then and elif [[ -n "$clear" ]]; then that was added in this PR seems to handle those cases correctly. I'd be happy to open a PR with that change.

lukaselmer pushed a commit to lukaselmer/ohmyzsh that referenced this pull request Apr 16, 2024
* Fixed a bug in *omz_history* where it would automatically create a file when run with the -p flag

* Reverted old history behaviour while fixing parsing bug
@mcornella
Copy link
Member

This is now fixed in #12357 thanks to @artnim. Cheers!

cschuyle pushed a commit to cschuyle/ohmyzsh that referenced this pull request Apr 18, 2024
* Fixed a bug in *omz_history* where it would automatically create a file when run with the -p flag

* Reverted old history behaviour while fixing parsing bug
cschuyle pushed a commit to cschuyle/ohmyzsh that referenced this pull request Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: core Issue or PR related to core parts of the project
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants