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

stop pasting leading spaces #4327

Closed
obfuscated opened this issue Aug 13, 2017 · 42 comments
Closed

stop pasting leading spaces #4327

obfuscated opened this issue Aug 13, 2017 · 42 comments

Comments

@obfuscated
Copy link

@obfuscated obfuscated commented Aug 13, 2017

Fish 2.6.0 on Gentoo linux, Terminology, no customizations.

The problem is that fish doesn't place every command in the history buffer. The last case happened when I had leading spaces in my commands. But most often this happens when there is some error in the parsing of the command or something that fish doesn't like.
I want and I'm used to that every time I press enter to execute something it enters the history butter, and I can bring it back pressing up arrow.

The simplest example is this one:

> echo "bla0"
bla0
> echo "bla1"
bla1
> echo "bla0" <-- this is what happens if I press the up-arrow key

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Aug 13, 2017

... happened when I had leading spaces in my commands.

This is expected behavior. It's borrowed from bash. Personally I don't like it. Also, fish has a way to switch to different history files and even disable history completely if you're doing something like a demo and don't want sensitive info from your normal command history to be visible.

But most often this happens when there is some error in the parsing of the command or something that fish doesn't like.

This is also by design and expected behavior.

The simplest example is this one:

I can't reproduce that behavior. And if that happened even infrequently I'm pretty confident we would know about it. If you run a few commands (including your echo examples) then run builtin history search -15 --show-time="%a %m-%d %R " does it show every command you ran?

@obfuscated
Copy link
Author

@obfuscated obfuscated commented Aug 13, 2017

@krader1961 The shell I'm using most of the time is bash, so I'm used to its behaviour and I'm complaining here because fish does unexpected and strange things that hinder my productivity.

Here is the same example done in bash 4.4.12(1)

$ echo "test0"
test0
$ echo "test1" <-- there are two spaces after the $
test1
$ echo "test1" <-- this is what happens if I press the up-arrow key

I have no idea if the history file and the list of commands accessed with up arrow are the same.
I don't see a reason why an executed command which has leading spaces in its string is not added to the list accessed with up-arrow. It should enter the history file, too, I don't know if it has to be trimmed or not, but I don't care.

FIY: I get these leading spaces by accident after I've remove something at the start of the command, but I've forgotten to remove the spaces. So this behaviour is 100% non-intentional and 100% surprising.

dir > echo "bla0"
bla0
dir > echo "bla1"
bla1
dir > echo "bla2" <-- leading space before echo
bla2
dir > echo "bla3" <-- leading space before echo
bla3
dir > builtin history search -4 --show-time="%a %m-%d %R "
Sun 08-13 21:42 echo "bla1"
Sun 08-13 21:42 echo "bla0"
Sun 08-13 21:42 builtin history search -3 --show-time="%a %m-%d %R "
Sun 08-13 21:41 builtin history search -e --show-time="%a %m-%d %R "

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Aug 13, 2017

From the documentation:

Prefixing the commandline with a space will prevent the entire line from being stored in the history.

Suggestions on how to make this more discoverable are welcome. The fish behavior is based on this section of the bash man page:

   HISTCONTROL
         A colon-separated list of values controlling how commands are saved  on  the
         history list.  If the list of values includes ignorespace, lines which begin
         with a space character are not saved  in  the  history  list.

Apparently enough people converting from bash like this feature it convinced the fish devs to make it the default. It's been discussed a couple of times in the past 1.5 years that I've been contributing to the fish project. And each time the outcome was to retain the current behavior. Having said that I agree with you that it is surprising and hard to discover why it occurs. It can't be changed in our next, and final, 2.x release. But we can discuss changing it for the upcoming 3.0.0 release. Especially since we now have the FISH_HISTORY mechanism which is a more robust and easier method of ensuring sensitive commands aren't inadvertently disclosed.

@krader1961 krader1961 added this to the fish-3.0 milestone Aug 13, 2017
@krader1961 krader1961 changed the title Not all executed commands are saved in the history stop treating leading spaces as an indicator not to save a command in the history file Aug 13, 2017
@obfuscated
Copy link
Author

@obfuscated obfuscated commented Aug 13, 2017

Interesting. It seems this is the default on Ubuntu. In Gentoo it is not. I'm not 100% sure about CentOS, but I'll check as soon as I get to my work machine.

@mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Aug 14, 2017

I would be very against changing this behavior, there's a valid use case (though questionable practice) for this in keeping secrets out of your history file. For example, when setting an environment variable access key or, more often, specifying the password for a connection as an argument (mysql -p, I'm looking at you!).

It's one of those things that you almost never use, almost never hurt you, but when you need it, you're glad it's there. How many complaints about this have we had over the years?

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Aug 14, 2017

How many complaints about this have we had over the years?

Probably impossible to get even a SWAG that's within an order of magnitude of the actual number. But I know that I was surprised by it the first time I encountered it and continue to be surprised for the same reason @obfuscated mentioned: If I accidentally leave leading whitespace as an artifact of editing a command the command doesn't get saved. Too, there have been a few discussions about this in the 1.9 years I've been using fish because others have noticed the same surprising behavior.

How many people know that this feature exists? I'm willing to bet real money that if we could survey our user base it would be single digit percentages. Very likely less that 5%. The main problem is that it is both obscure (hard to discover), surprising, and hard to debug. It is very much a "power" feature used by an insignificant percentage of our user base.

It is true that until recently, see issues #102 and #1504, there wasn't a viable alternative to not storing sensitive information in the command history. But as of the upcoming fish 2.7.0 release we have the FISH_HISTORY variable which you can change on the fly, or before launching a fish shell, to control where command history is written. If you're going to be running commands you don't want saved in the history just launch a fish subshell with it set to the empty string to not save any commands you type in that shell or a special prefix to use a history file that contains only sensitive command history. Then exit that subshell to get back to normal command history.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Aug 14, 2017

Also, you might be wondering how to know if you're not using the normal command history. After all, if I do something to enter that special history logging subshell I might forget that I have done so and find that non-sensitive commands are no longer being retrieved from, or saved to, my usual history. Much to my annoyance.

The answer is simple, use a custom fish_prompt function which provides an indication when FISH_HISTORY is not set to the default (fish) value. Then you'll be reminded when you're in a special subshell that isn't logging commands to your normal history file. This might even be something we want to do by default. Like we do for the fish Vi mode indicator via the fish_mode_prompt function to let the user know whether they are in Vi insert or command mode.

@mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Aug 14, 2017

I just feel like that's such a complicated solution to that question. Maybe I'm mistaken, but part of what makes me firm in my resolve is that I don't see a missing history as a big deal (when there exists an explanation for it, so it's not some phantom bug). So you trade the minor irritation of not having a whitespace-artifacted command in your history for the ability to just prefix with a single space anything you don't want in the history buffer.

A suggestion that might mitigate the problem: bracketed paste can trim leading and trailing whitespace.

I've had accidental whitespace artifacts mean my terminal history was incomplete myself, and off the top of my head, I think it's never happened except as a result of copy-and-paste (it's not like I accidentally typed a leading space). Would that take care of your accidental whitespace scenarios too, @krader1961 ?

@faho
Copy link
Member

@faho faho commented Aug 14, 2017

@obfuscated: Note that, in this example:

> echo "bla0"
bla0
> echo "bla1"
bla1
> echo "bla0" <-- this is what happens if I press the up-arrow key

I'm willing to bet that echo "bla1" appears as a suggestion (greyed-out in the commandline), which stops it from appearing in history navigation. See #405.

@obfuscated
Copy link
Author

@obfuscated obfuscated commented Aug 14, 2017

@faho In my version the bla1 line looks exactly the same as bla0. So you're losing this virtual bet. :)

@faho
Copy link
Member

@faho faho commented Aug 14, 2017

@obfuscated: What I mean is: After executing those commands, start typing echo "b. It should show a greyed out la1" after that, in the commandline. Press up-arrow. It should directly jump to echo "bla0". Clear the commandline and start typing echo "b. When the suggestion is shown press right-arrow, it should accept the la1".

@obfuscated
Copy link
Author

@obfuscated obfuscated commented Aug 14, 2017

@faho OK, You're correct, but I think it is unrelated to the issue. I'm not typing echo, I'm pressing up arrow and I'm surprised by the behaviour.

@faho
Copy link
Member

@faho faho commented Aug 14, 2017

Yes, but as soon as something from history is selected for the suggestion, it is excluded from history-navigation.

Or are you pressing up-arrow before that shows up?

@obfuscated
Copy link
Author

@obfuscated obfuscated commented Aug 14, 2017

@faho I execute a command. It fails or I cancel it, because I want to change something. The command produces tons of output. I press up-arrow (on an empty) line and I get a command I'm not expecting. This is very common behaviour of fish.

@mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Aug 14, 2017

@obfuscated I think you meant "I execute a command that has leading whitespace..." as it is a very specific case.

Like mentioned before, this is also how bash (the most popular and ubiquitous shell apart from sh) behaves.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Aug 14, 2017

This is very common behaviour of fish.

Not for me and I suspect not for anyone else. So the question is what is going on in your case. Unless, of course, you're running commands that have at least one leading space. In which case we know what the reason is.

@faho
Copy link
Member

@faho faho commented Aug 14, 2017

This isn't about the case where you've cancelled something. That's different, but not what I'm talking about right now.

When the command runs to completion, the behavior on an empty line is that up-arrow gives you the command. But once you type a prefix of it, it'll be selected for the suggestion and then up-arrow will give you something different. The idea is that you can use right-arrow to accept the suggestion, so you don't need another way to do the same.


Now, a command that just "fails" (i.e. returns a status != 0) still seems to be included in the history. If you're saying something different, I'd love to have a way to reproduce it.


If a command has a syntax error, it won't even allow you to execute it, and the commandline will stay as it is. So it's never entered into history, but there's no need to navigate to it because it's still there.


What I suspect is one of your pain points is that our ctrl-c binding doesn't enter the command into history - it just abandons the commandline. That is something we could change.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Aug 14, 2017

Would that take care of your accidental whitespace scenarios too, @krader1961 ?

Most but not all of them. It happens very infrequently but I have inadvertently ended up with a leading space. Also, it's not just pasting with the mouse. It's also pasting with [ctrl-V] (or [cmd-V]) that can result in a leading space. I guess the main problem with this behavior is that it is mystifying till you learn why it's happening. And that's hard to do because it's hard to discover.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Aug 14, 2017

I'd actually be happier if we had something explicit like a nohist command you could prefix to the real command to mark it as not being logged. Or perhaps command --nohist .... Anything but an all but invisible leading space.

Update: Obviously it's possible to have something short like nohist without introducing a new builtin and/or parser keyword. Just implement this as command --nohist and the user can do abbr nohist command --nohist or use any other short token they prefer; e.g., abbr = command --nohist.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Aug 14, 2017

Like mentioned before, this is also how bash (the most popular and ubiquitous shell apart from sh) behaves.

Except that it is configurable and off by default unless the distro has enabled it. For example, bash on macOS does not enable this as default behavior.

@anordal
Copy link
Contributor

@anordal anordal commented Aug 14, 2017

I often miss a command to forget the last history entry, because it was a mistake, and I don't want it suggested again. My command line often looks like this:

> git hceckout master
git: 'hceckout' is not a git command. See 'git --help'.
> git checkot master
git: 'checkot' is not a git command. See 'git --help'.
> git chekcout master
git: 'chekcout' is not a git command. See 'git --help'.
> git checkotu master
git: 'checkotu' is not a git command. See 'git --help'.

I'm also a heavy user of the space prefix. I don't know if I'm paranoid, but I always put a space before rm and other destructive commands unless I plan to repeat it. And that's pretty much muscle memory from bash!

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Aug 14, 2017

@anordal, Try history delete 'git hc' or just history delete git; although the latter is likely to match hundreds of history entries.

@ThomasAH
Copy link

@ThomasAH ThomasAH commented Sep 6, 2017

In GNU bash, version 4.4.12(1) on Debian stretch a leading space or an error still keeps the corresponding input line in the history but the default /etc/skel/.bashrc now contains HISTCONTROL=ignoreboth to enable this feature.
The comment in changelog.Debian.gz is just:

  * Merge from Ubuntu:
    - debian/skel.bashrc: Set HISTCONTROL to ignoreboth.

CentOS 7 only sets HISTCONTROL=ignoredups.

The upstream package of bash 4.4 does not set HISTCONTROL at all, not even in an example file.

So fish's current behaviour is surprising to many people, including me.
But if there at least would be a way to configure it, it would be OK for me.

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Sep 9, 2017

I think any behavior fish chooses here will be surprising to many people.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Sep 9, 2017

I think any behavior fish chooses here will be surprising to many people.

Yes, but the problem with the leading space solution is that it is hard to recognize and discover. Using something more explicit will annoy bash refugees who expect to be able to just preface their command with a space to keep it from ending up in the history. But it will be considerably more friendly to people who are not aware of that behavior. Especially since it is not the bash default behavior unless you're on a distro which enables it by default. And with our command abbreviation and key binding facilities people can define a single letter prefix even if we implement a more verbose "nohist" like command. That seems like the better trade-off to me.

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Sep 9, 2017

Those are valid points, however the leading space trick is more commonly known and easier to remember than some new invented syntax. Also TBH I am skeptical that "I accidentally added a leading space and then I tried to up-arrow that command" is common - seems like an unlikely set of factors.

How about having leading space act as a "don't write this to disk" modifier? Said commands would still be in your current session, but only in memory - once the session is closed they will be forgotten. This seems like it would address the "can't up-arrow" issue, while also providing a simple and familiar mechanism to retain secrets.

@obfuscated
Copy link
Author

@obfuscated obfuscated commented Sep 9, 2017

@ridiculousfish This (if it is enabled by default) will make it even harder to discover why entries to the history aren't saved. Just add an option to disable it and consider making it disabled by default. I don't think this is so serious problem which requires the invention of new syntax and other complex solutions...

@zanchey
Copy link
Member

@zanchey zanchey commented Sep 9, 2017

This has come up a lot - see #1383, #2118, #1606 and #615.

@ThomasAH
Copy link

@ThomasAH ThomasAH commented Sep 9, 2017

For me the spaces don't appear by accident, but from copy&paste from our internal documentation, which has all commands indented by two spaces. So when I want to do the same thing e.g. on the next day, I have to consult the documentation again.

Having the option to disable it would be good enough for me.

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Sep 10, 2017

Options for this sort of thing is against the fish design philosophy - see Configurability is the root of all evil. The question is whether we keep, modify, or remove the behavior.

One question I have is how do people keep stumbling onto this? I understand the pasting commands from docs issue and agree it's annoying, but that can't be a common source of leading spaces - I think?

@ThomasAH
Copy link

@ThomasAH ThomasAH commented Sep 10, 2017

Options for this sort of thing is against the fish design philosophy - see Configurability is the root of all evil. The question is whether we keep, modify, or remove the behavior.

Then I'd vote to remove the feature.

  • It is surprising for many people
  • It is not the default for bash, and only the default for some Linux distributions
  • It only provides a small benefit for security:
    1. Since it isn't available everywhere, people should not rely on it.
    2. Since sensitive information will show up in the process list, having sensitive information on the command line is inherently insecure, and it may even prevent the user from looking into alternatives, resulting in shell scripts using this insecure mechanism at a later point. Developers may even not implement an alternative, because they say "just enter a space before the command to not get it logged".

One question I have is how do people keep stumbling onto this? I understand the pasting commands from docs issue and agree it's annoying, but that can't be a common source of leading spaces - I think?

I must admit that I don't know any other situation that happens to me.
Maybe some people are still using Space to reset the screen saver (and have no screen locker)?

@obfuscated
Copy link
Author

@obfuscated obfuscated commented Sep 10, 2017

@ridiculousfish As I've previous said this happened to me after I've removed the beginning of a command. In this particular case I think I've removed the env part in the command "env BLA=bla2 somecommand", but I forgot to remove the space, because of sloppiness or habit.

Your options at the moment are:

  1. remove the feature
  2. replace it with nohist and add a faq entry.
  3. do nothing
  4. add some visual clue when this mode of operation is active. Add nohist to the prompt or something like this...

@braham-snyder
Copy link

@braham-snyder braham-snyder commented Sep 19, 2017

One question I have is how do people keep stumbling onto this?

for me: copying and pasting indented fish code from within fish scripts and functions

edit: and just to clarify: like others, I also run into this when I forget or deliberately take the shortcut of (and later regret) not removing a leading space I've accidentally left while editing a command

@madblobfish
Copy link
Contributor

@madblobfish madblobfish commented Oct 17, 2017

Please keep this behavior or find a better alternative, nohist is worse in my opinion.
Most importantly I'd like a good reliable warning for the users of the current feature. Think about the users using the feature:

  • nightly git users always on latest commit
  • stable users
  • long term support users which jump many versions (for example from fish-2.4 to fish-3.2 or something)

As it stands now I'm for a visual clue and maybe printing a message about what is happening when first used. This would be a short message just like when auto completing directories (vanishes when the command is executed).

I think that nohist only adds complexity and is even more inconvenient. Adding the whitespace is already inconvenient, especially when the commandline to be executed had already been written and contains newlines.

Another point I can make is that I sometimes copy with whitespace by purpose and expect the whitespace to show up, so trimming is probably also not too good.

@madblobfish
Copy link
Contributor

@madblobfish madblobfish commented Oct 17, 2017

Just got an Idea, don't know if its good or bad:
How about an insert mode for passwords/secrets?
While activated this mode wouldn't print anything to the terminal: therefore hiding the secret for people looking at your screen.
This mode could also keep the comandline in history without the secret, maybe inserting a placeholder.

@mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Oct 18, 2017

This was my motivation for implementing the new read command syntax. You can simply use (read -i) anywhere you need to securely enter a password.

@cben
Copy link
Contributor

@cben cben commented Dec 17, 2017

Proposal: whether you change it or not, display a hint after executing a command with leading space, ideally explaining how to achieve the opposite:

fish>   accidentally indented
...output...
Note: command started with space, will be omitted from history.  Run 'history add-last-omitted' if you want it.

or:

fish>   secret command
...output...
Note: command started with space, fish now saves such command in history!  Run `history delete -n 1` if undesired.

(both history syntaxes fictional, delete doesn't currently allow -n 1 / --max 1 flag.)

@madblobfish's proposal of vanishing message while typing is I think even cooler, avoids need to explain how to undo.

@BarbzYHOOL
Copy link

@BarbzYHOOL BarbzYHOOL commented Jul 19, 2018

What would be nice is to have it as a config option, so we could disable that feature.

@BarbzYHOOL
Copy link

@BarbzYHOOL BarbzYHOOL commented Jul 28, 2018

Also I have written tons of tutorials for myself when I used bash, and I indented all the commands (a bit like we do in markdown on github)

and now everytime i copy them into fish, i deal with this issue :'(

@floam
Copy link
Member

@floam floam commented Dec 11, 2018

I think only ignoring commands with exactly one leading space would be a reasonable change. The accidental stuff from bad pastes tends to be a few spaces for me.

@ThomasAH
Copy link

@ThomasAH ThomasAH commented Dec 11, 2018

I think only ignoring commands with exactly one leading space would be a reasonable change. The accidental stuff from bad pastes tends to be a few spaces for me.

This would be fine for me (as written above, our internal documentation has two leading spaces), but still sounds quite magic/surprising.

@mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Dec 11, 2018

That is probably even more confusing. I think ignoring leading whitespace on paste is the correct approach, it seems to be the biggest cause of consternation.

faho added a commit that referenced this issue Mar 13, 2019
Similar to the last commit, only for the in-terminal-paste stuff.

Also cleans up the comments on bracketed paste a bit - nobody has
stepped forward to report problems with old emacsen or windows, so
there's no need for a TODO comment.

See #4327.
@faho faho removed this from the fish-future milestone Mar 26, 2019
@faho faho added this to the fish 3.1.0 milestone Mar 26, 2019
@faho faho changed the title stop treating leading spaces as an indicator not to save a command in the history file stop pasting leading spaces Mar 26, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests