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

ui: add bell notification (beep) #945

Closed
wants to merge 1 commit into from

Conversation

joukewitteveen
Copy link

The bell is sounded when there is nothing to undo/redo.
This is consistent with vim, although vim has many more beeps cases.


I wanted to browse the vis code-base a bit and decided that this was a good excuse to do so.
In VIM, the bell can be silenced by configuring a "visual bell" (screen flash) and setting the visual bell command to the empty string. I reckon implementing anything like a "visual bell" is out of scope for vis (it can be configured in your terminal emulator anyway), but we might want an option to silence the bell still.

@joukewitteveen joukewitteveen force-pushed the feature-bell branch 2 times, most recently from bdced70 to 8c6649b Compare April 25, 2021 17:33
@mcepl mcepl mentioned this pull request Aug 4, 2023
8 tasks
@rnpnr
Copy link
Collaborator

rnpnr commented Aug 4, 2023

I don't see anything wrong with this code but besides this pull request
no one is complaining about vis' lack of a terminal bell. Personally I
really don't like it so I'm not interested in applying this.

But, since this isn't rnpnr/vis I'm not closing this and will let
others decide. Most people probably won't want to comment so
you can leave a reaction to this post vote.

👍 : I agree DON'T merge this.
👎 : I disagree DO merge this.

@mcepl
Copy link
Contributor

mcepl commented Aug 28, 2023

My other problem with this PR is that it does more than what’s in the commit message. It seems to be playing with the colour palette more than just what’s necessary to introducing the bell notification (at least unnecessary formatting changes).

@joukewitteveen
Copy link
Author

unnecessary formatting changes

There are two places where I put the { on the same line as the definition for consistency. Other apparent unnecessary changes are mainly just git not showing the diff the way you should think about it. There is no actual changing of colour palette code going on.

For me, the addition of a bell is a matter of accessibility as it allows me to get into a known state without looking at the screen: repeat until the bell sounds and you know where you are.

@rnpnr
Copy link
Collaborator

rnpnr commented Aug 28, 2023

the addition of a bell is a matter of accessibility

Maybe I'm ignorant but is it possible to use vis without looking at
the screen? If that's the case there is a possibility that I can be
convinced. Otherwise I think I will close this.

The bell is sounded when there is nothing to undo/redo.
This is consistent with vim, although vim has many more beep cases.
@joukewitteveen
Copy link
Author

is it possible to use vis without looking at the screen?

Sure! I'm a confident typist and can for instance transcribe or translate something without looking at either the screen or keyboard. With vim, the bell is really useful for such use cases. With vis the proposed change is actually very minor: it just sounds the bell at the end of the undo (and redo) stack. This allows you to quickly undo all changes by hammering u a few times until the bell sounds.

@erf
Copy link
Contributor

erf commented Aug 28, 2023

With an option to silence the bell (defaulted to off), i don't mind this change.

@joukewitteveen
Copy link
Author

With an option to silence the bell (defaulted to off), i don't mind this change.

I would consider that useless code that serves as nothing but a maintenance burden. The audible aspect of the terminal bell can be configured in your terminal emulator. The bell is sounded in many more prevalent circumstances than at the end of undo/redo sequences in your editor. Tab completion for example also typically sounds the bell. Have you actually tried vis with the change? The change is fairly minimal.

@erf
Copy link
Contributor

erf commented Aug 28, 2023

With an option to silence the bell (defaulted to off), i don't mind this change.

I would consider that useless code that serves as nothing but a maintenance burden. The audible aspect of the terminal bell can be configured in your terminal emulator. The bell is sounded in many more prevalent circumstances than at the end of undo/redo sequences in your editor. Tab completion for example also typically sounds the bell. Have you actually tried vis with the change? The change is fairly minimal.

Yes I've tried it and don't mind the default bell sound, especially when i see i can configure my terminal (Wezterm) on how to respond to it. Not sure all users agree though .. and it does require some additional effort by the user to return to the previous behavior

@mcepl
Copy link
Contributor

mcepl commented Feb 14, 2024

I think we need a decision about this issue, @rnpnr , either merge this or reject it, but this lingers here.

@rnpnr
Copy link
Collaborator

rnpnr commented Mar 10, 2024

It looks to me like most people do not want this patch. For the time being it will have to remain local for anyone who wants it. Feel free to link it in the wiki if you want.

@rnpnr rnpnr closed this Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants