Skip to content

Remove read history, let history delete read search terms interactively#5904

Merged
faho merged 3 commits into
fish-shell:masterfrom
faho:read-history
Jun 6, 2019
Merged

Remove read history, let history delete read search terms interactively#5904
faho merged 3 commits into
fish-shell:masterfrom
faho:read-history

Conversation

@faho
Copy link
Copy Markdown
Member

@faho faho commented May 29, 2019

Description

This removes builtin reads history, which is:

  • undocumented
  • possibly unexpected (we've recommended mysql -p(read) before, with the intention that these not show up in the history)

Then it uses read in history delete if no searchterms are given.

An alternative would be to add a flag to read to not keep history (read --incognito or read --no-history?), or to readd the --mode-name flag and expect that. I've not done so because I don't see the use in the read history, and I believe that the expectation usually is that it doesn't keep history.

Fixes issue #5791

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.md

faho added 3 commits May 29, 2019 21:48
This was undocumented, not all that useful and potentially unwanted.

In particular it means that things like

   mysql -p(read)

will still keep the password in history.

Also it allows us to simply implement asking for the history deletion
term.

See fish-shell#5791.
@faho faho added this to the fish 3.1.0 milestone May 29, 2019
@faho faho added the RFC label Jun 1, 2019
@ridiculousfish
Copy link
Copy Markdown
Member

Convinced me - let's do it. LGTM.

If this proves controversial we can add it back as an opt-in, it ought not to be the default.

@faho faho merged commit ae59fde into fish-shell:master Jun 6, 2019
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 16, 2020
@faho faho deleted the read-history branch September 23, 2021 09:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants