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

fish_bell option to enable or disable bell on reader_t::flash(). #7875

Closed
wants to merge 1 commit into from

Conversation

ewtoombs
Copy link
Contributor

@ewtoombs ewtoombs commented Mar 28, 2021

fish_bell's behaviour is documented in doc_src/language.rst.

Description

The option was added because visual (and especially audio) feedback after
failed user interaction is usually unnecessary and can be extremely annoying.
It is only useful over bad networks, when it isn't clear whether the
interaction failed or there is network lag.

The terminal bell is still useful for other things, like notifying on the completion of a long command, so disabling the terminal bell entirely is not preferable to many users.

TODOs:

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

fish_bell's behaviour is documented in doc_src/language.rst.

The option was added because visual (and especially audio) feedback after
failed user interaction is usually unnecessary and can be extremely annoying.
It is only useful over bad networks, when it isn't clear whether the
interaction failed or there is network lag.
@ridiculousfish
Copy link
Member

Thank you for the PR. Instead of adding configuration, I think we should just remove the bell entirely - I doubt anyone will miss it. Does anyone like the bell and want to keep it?

@faho
Copy link
Member

faho commented Mar 29, 2021

Let's kill it.

@faho faho closed this in 312cfac Mar 29, 2021
@faho
Copy link
Member

faho commented Mar 29, 2021

Okay I just removed the bell entirely, it's not actually a good idea in this context.

@ewtoombs
Copy link
Contributor Author

Hm. Yep, works for me. Thanks, @faho and @ridiculousfish !

@floam
Copy link
Member

floam commented Jul 19, 2021

l liked the bell, (I added it) but ya snooze, ya lose.

@ewtoombs
Copy link
Contributor Author

l liked the bell, (I added it) but ya snooze, ya lose.

I'd still like to know your rationale.

@floam
Copy link
Member

floam commented Jul 22, 2021

#2755

@floam
Copy link
Member

floam commented Jul 22, 2021

The little audible ding or the terminal flashing with a visual bell is a decent stimuli to let one know they're about to attempt an invalid command. Just like if one attempts an invalid keyboard shortcut in the rest of the OS. I think it is the most right way to give the user that kind of feedback.

Sending a BEL is a better modality than the flashy thing we faked, as terminals can implement it in the most rich way possible, and can take into account accessibility requirements (usually configurable as either audible or visual).

@floam
Copy link
Member

floam commented Jul 22, 2021

Also, I really do not understand "It is only useful over bad networks".

@ewtoombs
Copy link
Contributor Author

ewtoombs commented Jul 28, 2021

Invalid keyboard shortcuts elsewhere in the operating system do not produce audible bells. The fact that nothing happens is all the stimulus a user needs to be able to tell something doesn't work. Anything extra is just spurious and annoying. Even the terminal flash is unnecessary and annoying, but not nearly as annoying as the bell, so I don't mind if others want to keep that.

A general rule of UI development is never to produce audible noises in direct response to keypresses. Modern design overwhelmingly follows this rule. This is not the way UIs have always been, and they are so much better for having made this change. Adding audible bells is a massive step backward from this advance.

@ewtoombs
Copy link
Contributor Author

ewtoombs commented Jul 28, 2021

This idea of preventing the running of invalid commands is new to me, though. The flash does correlate with a failed history search and when that happens, the command will be invalid. If somebody does a search assuming it will succeed, then immediately presses enter without checking the command, they can accidentally enter (potentially dangerous) nonsense into the terminal. The flash does steer people away from that. That is good. So, the flash is doing something useful. I can agree with that. Just the flash is plenty though.

Personally, the lack of response to a history search is more than enough stimulus for me to tell that the search failed, so the flash doesn't do anything for me, stimulus-wise, but I can see how others would find it helpful.

@ewtoombs
Copy link
Contributor Author

You may rightly ask why I don't just switch to a visual terminal bell. That's because I use the audible terminal bell to notify me when a long command is complete. It is very useful in this regard. I mentioned this before. I even have an alias for this:

alias bell='echo -n \a'

Example:

> rm -rf huge_freaking_folder; bell

It is so useful that it would actually be really cool if fish automatically rang the terminal bell at completion if a command has taken more than, say, 5 seconds and is still running. But I digest.

@ewtoombs
Copy link
Contributor Author

ewtoombs commented Jul 28, 2021

Also, I really do not understand "It is only useful over bad networks".

Right, let me unpack this one lol. I can see that I didn't really explain this completely.

Suppose that we're in an alternate universe where fish notifies the user of failed history searches by doing nothing at all. So, no bell and no flash either.

Suppose you're in an SSH session and your network is total garbage. You want to do ps aux | grep nginx. On a new line, you type ps ^P. Suppose the search will succeed, but nothing happens initially, due to network lag. You might now accidentally assume the search failed, and start typing out the full command. When the network kicks back in, the history search succeeds, and everything you have typed so far is inserted at the end of the line, after the correct command. Now, you are forced to erase what you have added to the end of the command or start over. From this experience, you have learned that you have to wait a few seconds after every history search to make sure it has really failed.

The flash solves this problem nicely, because you can use it to discern network lag from a failed search. But I don't consider it a priority, because there are a whole bunch of other instances where you'd also have to add flashes in order for fish to be robust against bad networks—something fish hasn't done. So it's pretty clear to me that network lag is not a design consideration in fish. I actually think this is totally fine. Bad networks are so rare these days that designing for it doesn't really make sense any more.

@faho
Copy link
Member

faho commented Jul 28, 2021

@ewtoombs Could you take this elsewhere? I get notified for every message in here.

@ewtoombs
Copy link
Contributor Author

@ewtoombs Could you take this elsewhere? I get notified for every message in here.

With an audible bell, or with a flash? :D

Right, @floam, you can /msg me on libera if you want to keep discussing this. My nick is triteraflops.

Sorry, @faho.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants