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

Allow `read` to not store history #1504

Closed
xfix opened this Issue Jun 12, 2014 · 8 comments

Comments

Projects
None yet
4 participants
@xfix
Member

xfix commented Jun 12, 2014

Sometimes, the user may want to read without storing history. However, there is no option to not store the history.

@xfix xfix added this to the fish-future milestone Sep 21, 2014

@xfix xfix added the enhancement label Sep 21, 2014

@Asuza

This comment has been minimized.

Show comment
Hide comment
@Asuza

Asuza Oct 29, 2015

@xfix, does this only happen from the command line? I used read in a script and it didn't seem to store that history anywhere (as far as I know).

Asuza commented Oct 29, 2015

@xfix, does this only happen from the command line? I used read in a script and it didn't seem to store that history anywhere (as far as I know).

@cben

This comment has been minimized.

Show comment
Hide comment
@cben

cben Feb 12, 2016

Contributor

Aha, found it!
It's not stored in regular shell history but read has its own readline history, which can be accessed by up arrow when you run read again.
It's even persisted (~/.config/fish/fish_read_history) and shared immediately among fish instances.
There is already an option that controls it:

  • -m NAME or --mode-name=NAME specifies that the name NAME should be used to save/load the history file. If NAME is fish, the regular fish history will be available.

but even empty --mode-name="" doesn't disable storing (it writes to ~/.config/fish/_history).

👍, one of the use cases of read is entering passwords without storing them anywhere, and this subverts that...
Obviously once silent mode (#838) is implemented, it should also disable storing.

There should probably also be an independent flag to control storing. (Or redefine --mode-name=""? IMHO that'd be hard to discover; seeing something like --no-history when scanning the --help would make it clearer that it is stored by default.)

Contributor

cben commented Feb 12, 2016

Aha, found it!
It's not stored in regular shell history but read has its own readline history, which can be accessed by up arrow when you run read again.
It's even persisted (~/.config/fish/fish_read_history) and shared immediately among fish instances.
There is already an option that controls it:

  • -m NAME or --mode-name=NAME specifies that the name NAME should be used to save/load the history file. If NAME is fish, the regular fish history will be available.

but even empty --mode-name="" doesn't disable storing (it writes to ~/.config/fish/_history).

👍, one of the use cases of read is entering passwords without storing them anywhere, and this subverts that...
Obviously once silent mode (#838) is implemented, it should also disable storing.

There should probably also be an independent flag to control storing. (Or redefine --mode-name=""? IMHO that'd be hard to discover; seeing something like --no-history when scanning the --help would make it clearer that it is stored by default.)

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Feb 12, 2016

Again, this is one of those things that I think should belong in userland.

ghost commented Feb 12, 2016

Again, this is one of those things that I think should belong in userland.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Feb 12, 2016

Contributor

Just do read -m /dev/null var.

Contributor

krader1961 commented Feb 12, 2016

Just do read -m /dev/null var.

@cben

This comment has been minimized.

Show comment
Hide comment
@cben

cben Feb 22, 2016

Contributor

@bucaran: Oh I see, ... | read var does not store any history. I suppose stdin is not a tty so it skips the history/readline code path.
I don't think that's obvious from help read, will try to impove the doc as a starting point.

@krader1961 That happens to work but not quite intentionally, -m doesn't expect a full path. strace:

open("/home/bpaskinc/.config/fish//dev/null_history.o8WTAF", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.SIk6NR", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.M1Ti13", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.FuFveg", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.JdDIrs", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.69MVEE", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.2u88RQ", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.0vFm52", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.XgoAif", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.0hmOvr", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
Contributor

cben commented Feb 22, 2016

@bucaran: Oh I see, ... | read var does not store any history. I suppose stdin is not a tty so it skips the history/readline code path.
I don't think that's obvious from help read, will try to impove the doc as a starting point.

@krader1961 That happens to work but not quite intentionally, -m doesn't expect a full path. strace:

open("/home/bpaskinc/.config/fish//dev/null_history.o8WTAF", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.SIk6NR", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.M1Ti13", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.FuFveg", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.JdDIrs", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.69MVEE", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.2u88RQ", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.0vFm52", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.XgoAif", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
open("/home/bpaskinc/.config/fish//dev/null_history.0hmOvr", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = -1 ENOENT (No such file or directory)
@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Feb 23, 2016

Contributor

That read -m /dev/null treats the path as a simple word used in constructing a path is arguably a bug. It's certainly the most natural way of telling it not to use a history file on a UNIX platform. Although the definition of the option says nothing about specifying a path name I think making /dev/null a special-case is reasonable. But we should not allow arbitrary paths; i.e., we should raise an error if the "name" includes a forward or backward slash. It would probably be a good idea to also allow an empty string to inhibit use of a history file.

Contributor

krader1961 commented Feb 23, 2016

That read -m /dev/null treats the path as a simple word used in constructing a path is arguably a bug. It's certainly the most natural way of telling it not to use a history file on a UNIX platform. Although the definition of the option says nothing about specifying a path name I think making /dev/null a special-case is reasonable. But we should not allow arbitrary paths; i.e., we should raise an error if the "name" includes a forward or backward slash. It would probably be a good idea to also allow an empty string to inhibit use of a history file.

@krader1961 krader1961 self-assigned this Feb 23, 2016

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Feb 23, 2016

It would probably be a good idea to also allow an empty string to inhibit use of a history file.

This would be perfect.

ghost commented Feb 23, 2016

It would probably be a good idea to also allow an empty string to inhibit use of a history file.

This would be perfect.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 Jul 1, 2017

Contributor

Now that issue #102 has been addressed, and we have the FISH_HISTORY var for choosing the history file, we should change read to honor that var and deprecate the --mode-name flag. By deprecate I mean "print a warning telling the user to use FISH_HISTORY and ignore the flag." There are no uses of the flag (or the -m short flag) in any fish, Fisherman, or OMF scripts.

Contributor

krader1961 commented Jul 1, 2017

Now that issue #102 has been addressed, and we have the FISH_HISTORY var for choosing the history file, we should change read to honor that var and deprecate the --mode-name flag. By deprecate I mean "print a warning telling the user to use FISH_HISTORY and ignore the flag." There are no uses of the flag (or the -m short flag) in any fish, Fisherman, or OMF scripts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment