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

fix: only add to history of different #8197

Open
wants to merge 2 commits into
base: mxyng/gods-v2
Choose a base branch
from

Conversation

mxyng
Copy link
Contributor

@mxyng mxyng commented Dec 21, 2024

if the last item in history is the same as the one being added, skip it. this reduces the number of history entries. the behaviour is similar to how most shells maintain history

if the last item in history is the same as the one being added, skip it.
this reduces the number of history entries. the behaviour is similar to
how most shells maintain history
@mxyng mxyng force-pushed the mxyng/cmd-history branch from 2951ebf to 77e7e12 Compare December 21, 2024 08:52
Comment on lines -119 to -124
if i.History.Pos > 0 {
if i.History.Pos == i.History.Size() {
currentLineBuf = []rune(buf.String())
}
buf.Replace([]rune(i.History.Prev()))
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the block below are unnecessary since History.Prev() and History.Next() should return the correct line content; the logic is duplicated between here and readline/history.go and it seems more appropriate for it to live there. The only benefit of having this conditional is omitting the call to Replace() which doesn't seem worth it for the added complexity


f, err := os.OpenFile(path, os.O_CREATE|os.O_RDONLY, 0o600)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be possible with O_CREATE

@mxyng mxyng force-pushed the mxyng/cmd-history branch 2 times, most recently from 8b0bf9e to 7570003 Compare December 21, 2024 09:08
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

Successfully merging this pull request may close these issues.

2 participants