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

Add command to remove entry from history. #11620

Open
Hofer-Julian opened this issue Jan 23, 2024 · 9 comments · May be fixed by #11629
Open

Add command to remove entry from history. #11620

Hofer-Julian opened this issue Jan 23, 2024 · 9 comments · May be fixed by #11629
Labels
enhancement New feature or request needs-triage An issue that hasn't had any proper look

Comments

@Hofer-Julian
Copy link
Contributor

Related problem

When using sqlite as history backend, users might find it hard to delete individual entries from history.

Describe the solution you'd like

It would be good if nushell could provide a command for that.
This command would take an index and deletes it from the history in memory and history in the sqlite db.

One possible name would be history remove-item.

Describe alternatives you've considered

Users could manually adapt the sqlite file

Additional context and details

@fdncred mentioned this issue as a blocker for enabling sqlite and history isolation per default for nushell: #10440 (comment)

@Hofer-Julian Hofer-Julian added enhancement New feature or request needs-triage An issue that hasn't had any proper look labels Jan 23, 2024
@fdncred
Copy link
Collaborator

fdncred commented Jan 23, 2024

This one was written in nu_script by @giggio #11470 (comment)
https://github.com/giggio/nuscripts/blob/main/scripts/history-command.nu

# Deletes a history entry.
def 'history delete' [
  id: int = 0 # the id of the history entry to delete
  --last # delete the last entry, ignore the id, if passed
  ] {
  if $last {
    open $nu.history-path | query db $"delete from history where id = \(select id from \(select id from history order by id desc LIMIT 2) order by id asc LIMIT 1)"
  } else {
    if $id == 0 {
      echo "You must pass an id or use --last"
      exit 1
    }
    open $nu.history-path | query db $"delete from history where id = ($id)"
  }
  null
}

@Hofer-Julian
Copy link
Contributor Author

Are you suggesting adding it to std, or did you just mention it for inspiration?

@fdncred
Copy link
Collaborator

fdncred commented Jan 23, 2024

Are you suggesting adding it to std, or did you just mention it for inspiration?

Either? I've never really used it, but it looks sound. Is this how you'd want a history remove-item to work?

@Hofer-Julian
Copy link
Contributor Author

Hofer-Julian commented Jan 23, 2024

Is this how you'd want a history remove-item to work?

I think so?
To be honest I don't fully understand what this does:
\(select id from \(select id from history order by id desc LIMIT 2) order by id asc LIMIT 1)

I've assumed simply grabbing the entry with the correct id would be enough

@fdncred
Copy link
Collaborator

fdncred commented Jan 23, 2024

ok.

ya, i'm not sure why the LIMIT 2 and then LIMIT 1. I try to stay away from nested queries. @gigggio i assume you're doing that for some specific reason that we aren't seeing. can you provide any insight?

@giggio
Copy link

giggio commented Jan 24, 2024

Yes, there is a reason. That is for deleting the last command. But the command you just entered became the last, so what you actually need to do is delete the one before last. That is what that does. As the first query is limiting to 2 rows only, I didn't see a problem to nest. If there is a better way to get the row before the last one I'd like to know, I can update my version, too.

And now that I'm saying that I realize that maybe you'd want to delete the last from your current session, and I'm not doing that filtering. I think I'll update my script. :)

@fdncred
Copy link
Collaborator

fdncred commented Jan 24, 2024

Please share your script if you decide to update it.

Hofer-Julian added a commit to Hofer-Julian/nushell that referenced this issue Jan 24, 2024
@Hofer-Julian Hofer-Julian linked a pull request Jan 24, 2024 that will close this issue
@fdncred
Copy link
Collaborator

fdncred commented Jan 24, 2024

nice! Thanks @giggio - just adding it here for fun. I understand why you do the query for --last now. Good work!

# Deletes a history entry.
def 'history delete' [
  id: int = 0 # the id of the history entry to delete
  --last # delete the last entry, ignore the id, if passed
  ] {
  if $last {
    open $nu.history-path | query db $"delete from history where id = \(select id from \(select id from history where session_id = (history session) order by id desc LIMIT 2) order by id asc LIMIT 1)"
  } else {
    if $id == 0 {
      echo "You must pass an id or use --last"
      exit 1
    }
    open $nu.history-path | query db $"delete from history where id = ($id)"
  }
  null
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-triage An issue that hasn't had any proper look
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants