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

ctrl+/ rings terminal bell even with "enable_audio_bell no" #4489

Closed
michaelsbradleyjr opened this issue Jan 9, 2022 · 33 comments
Closed

ctrl+/ rings terminal bell even with "enable_audio_bell no" #4489

michaelsbradleyjr opened this issue Jan 9, 2022 · 33 comments
Labels

Comments

@michaelsbradleyjr
Copy link

michaelsbradleyjr commented Jan 9, 2022

ctrl+/ rings the terminal bell on macOS even with enable_audio_bell no in kitty.conf. I have not tried it on Linux or other OS.

kitty 0.24.1 created by Kovid Goyal
Darwin elias.local 19.6.0 Darwin Kernel Version 19.6.0: Sun Nov 14 19:58:51 PST 2021; root:xnu-6153.141.50~1/RELEASE_X86_64 x86_64
ProductName:    Mac OS X ProductVersion:        10.15.7 BuildVersion:   19H1615
Frozen: True
Paths:
  kitty: /Applications/kitty.app/Contents/MacOS/kitty
  base dir: /Applications/kitty.app/Contents/Resources/kitty
  extensions dir: /Applications/kitty.app/Contents/Resources/Python/lib/kitty-extensions
  system shell: /usr/local/bin/bash
Loaded config files:
  /Users/michael/.config/kitty/kitty.conf

Config options different from defaults:
allow_remote_control    y
confirm_os_window_close 1
cursor_blink_interval   0.0
enable_audio_bell       False
font_size               14.0
listen_on               unix:${HOME}/temp/kitty/kitty
macos_option_as_alt     3
visual_bell_duration    0.1
Added shortcuts:
        shift+ctrl+/ launch --allow-remote-control kitty +kitten kitty_search/search.py @active-kitty-window-id
        cmd+k combine : clear_terminal scrollback active : send_text normal \x0c
        cmd+s send_text normal,application \x18@s
@michaelsbradleyjr michaelsbradleyjr changed the title ctrl+/ rings terminal bell even with "enable_audio_bell no`" ctrl+/ rings terminal bell even with "enable_audio_bell no" Jan 9, 2022
@page-down
Copy link
Contributor

I suspect that Cocoa rings when this key combination is pressed.

I tried to handle it in glfw/cocoa_init.m is_modified_special_key, but it still makes a sound.

How to remove NSResponder, NSBeep useless responses?

@kovidgoyal
Copy link
Owner

You probably need to implement performKeyEquivalent to get cocoa to not
be a pain. https://stackoverflow.com/questions/8869462/prevent-not-allowed-beep-after-keystroke-in-nsview

Implement it, call the super class method and return YES

@page-down
Copy link
Contributor

I handle it in GLFWContentView, if returns YES, it will not get the ctrl,cmd combination key press event. Does it mean that keypress events are not processed at this level and should not be terminated here (return yes)?

[super performKeyEquivalent:event];
return YES;

Another thing I noticed (without the above patch)

ctrl+d

Press: native_key: 0x2 (d) ... doCommandBySelector: (deleteForward:)

However ctrl+/ does not have any doCommandBySelector.
And it looks like ctrl+d should not call (deleteForward:).

@kovidgoyal
Copy link
Owner

kovidgoyal commented Jan 10, 2022 via email

@page-down
Copy link
Contributor

Are you saying, this patch kauses keyDown not be called?

Yes, only the ctrl+xxx, cmd+xxx modifier key combinations fails to call keydown.

It doesnt matter, none of Cocoa's builtin actions actually do anything in kitty, other than insertText.

Ok, as long as there are no performance issues, then it can be ignored.

@kovidgoyal
Copy link
Owner

Yes, performKeyEquivalent is called first, so you would need to
explicitly forward the events to keyDown and the menus if overriding it.
See https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/EventOverview/EventArchitecture/EventArchitecture.html#//apple_ref/doc/uid/10000060i-CH3-SW10

@page-down
Copy link
Contributor

page-down commented Jan 10, 2022

Thanks.
I tried to let keyDown handle it and not worry about the menu part for now.

// performKeyEquivalent:
[self keyDown:event]; return YES;

Then I noticed that the keys still made a sound. Emitted from a function call in keyDown.

// keyDown:
[self interpretKeyEvents:[NSArray arrayWithObject:event]];
// ... sends the character input in eventArray to the system input manager
// for interpretation as text to insert or commands to perform ...

If we want to implement this function and send key events to the menu. Then we need to handle only the shortcuts registered in the kitty and send all others to the menu or the system. Otherwise all the "global shortcuts" configured in the system will not work.

I have not yet solved the problem, here are my notes.


It does not make a sound in iTerm2, but this is a problem that exists in many other programs. It has been mentioned in numerous other open source project issues.

some key combinations that make a sound:

ctrl+cmd+down -> ((void)moveDown:(id)sender;)
ctrl+cmd+left/right
ctrl+/
...

I have tested the following and it seems that none of them(in NSView, NSWindow, NSApp) will be called after implementation.

// Responder Chain for Event Messages
...
If no object is found to handle the event,
the last responder in the chain invokes noResponderFor:,
which for a key-down event simply beeps.

https://developer.apple.com/documentation/appkit/nsresponder/1534197-noresponderfor

- (void)noResponderFor: (SEL) selector
{
    if (selector != @selector(keyDown:)) [super noResponderFor:selector];
}

Workaround:
~/Library/KeyBindings/DefaultKeyBinding.dict

https://apple.stackexchange.com/questions/23981/command-control-arrow-beeps-plays-alert-sound-in-lion

@kovidgoyal
Copy link
Owner

why not simply override noResponderFor and if the event is a keyDown
dont call the super

@page-down
Copy link
Contributor

why not simply override noResponderFor and if the event is a keyDown
dont call the super

Yes, I edited the above before you replied.

@interface GLFWContentView : NSView <NSTextInputClient>
@implementation GLFWContentView

@interface GLFWWindow : NSWindow
@implementation GLFWWindow

@interface GLFWApplication : NSApplication
@implementation GLFWApplication

At this point noResponderFor is not called and the sound is emitted from keyDown -> interpretKeyEvents.

@kovidgoyal
Copy link
Owner

In that case figure out what the last responder is using this technique

https://stackoverflow.com/questions/4241416/how-to-inspect-the-responder-chain

and see if you can override it there.

@kovidgoyal
Copy link
Owner

@page-down
Copy link
Contributor

I think kitty use a very simple layout with almost nothing.
When ctrl+/, ctrl+cmd+left are pressed.

Responder chain:
  <GLFWContentView: ...>
  <GLFWWindow: ...>

I have implemented noResponderFor in all three of the above mentioned Objects. But none of these will be called. I can see that they are called when I move the window with the mouse.

It looks like interpretKeyEvents doesn't care about this function?

@kovidgoyal
Copy link
Owner

if its happening via interpretkeyevents then this technique should do
the trick: https://christiantietze.de/posts/2016/11/nsresponder-finding-beep/

@page-down
Copy link
Contributor

Yes, I pulled out lldb and found this NSBeep.

ctrl+/

  * frame #0: 0x00007ff803a7b809 AppKit`NSBeep
    frame #1: 0x00007ff803a43de0 AppKit`-[NSTextInputContext(NSInputContext_WithCompletion) doCommandBySelector:completionHandler:] + 177
    frame #2: 0x00007ff80396e37e AppKit`-[NSKeyBindingManager(NSKeyBindingManager_MultiClients) interpretEventAsCommand:forClient:] + 2271
    frame #3: 0x00007ff803976eb1 AppKit`__84-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:]_block_invoke_5 + 533
    frame #4: 0x00007ff804121a67 AppKit`__84-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:]_block_invoke_3.843 + 74
    frame #5: 0x00007ff803976c71 AppKit`-[NSTextInputContext tryHandleEvent_HasMarkedText_withDispatchCondition:dispatchWork:continuation:] + 87
    frame #6: 0x00007ff8041219ed AppKit`__84-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:]_block_invoke.840 + 251
    frame #7: 0x00007ff80999d1f1 HIToolbox`__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke_5 + 70
    frame #8: 0x00007ff8099ae4ae HIToolbox`invocation function for block in DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 110
    frame #9: 0x00007ff80411d112 AppKit`__55-[NSTextInputContext handleTSMEvent:completionHandler:]_block_invoke.310 + 885
    frame #10: 0x00007ff8039701a8 AppKit`__55-[NSTextInputContext handleTSMEvent:completionHandler:]_block_invoke_2 + 74
    frame #11: 0x00007ff80397012e AppKit`-[NSTextInputContext tryHandleTSMEvent_HasMarkedText_withDispatchCondition:dispatchWork:continuation:] + 87
    frame #12: 0x00007ff80396f689 AppKit`-[NSTextInputContext handleTSMEvent:completionHandler:] + 1859
    frame #13: 0x00007ff80396eed8 AppKit`_NSTSMEventHandler + 299
    frame #14: 0x00007ff80993ed1d HIToolbox`DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 1391
    frame #15: 0x00007ff80993e14e HIToolbox`SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) + 333
    frame #16: 0x00007ff80993dfef HIToolbox`SendEventToEventTargetWithOptions + 45
    frame #17: 0x00007ff809999666 HIToolbox`SendTSMEvent_WithCompletionHandler + 381
    frame #18: 0x00007ff809999b11 HIToolbox`__SendUnicodeTextAEToUnicodeDoc_WithCompletionHandler_block_invoke + 387
    frame #19: 0x00007ff809999969 HIToolbox`__SendFilterTextEvent_WithCompletionHandler_block_invoke + 182
    frame #20: 0x00007ff8099996b4 HIToolbox`SendTSMEvent_WithCompletionHandler + 459
    frame #21: 0x00007ff8099994bc HIToolbox`SendFilterTextEvent_WithCompletionHandler + 219
    frame #22: 0x00007ff809999195 HIToolbox`SendUnicodeTextAEToUnicodeDoc_WithCompletionHandler + 274
    frame #23: 0x00007ff809998f45 HIToolbox`__utDeliverTSMEvent_WithCompletionHandler_block_invoke_2 + 281
    frame #24: 0x00007ff809998da5 HIToolbox`__utDeliverTSMEvent_WithCompletionHandler_block_invoke + 354
    frame #25: 0x00007ff809998bf8 HIToolbox`TSMKeyEvent_WithCompletionHandler + 577
    frame #26: 0x00007ff80999899c HIToolbox`__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke_4 + 250
    frame #27: 0x00007ff80999881b HIToolbox`__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke_3 + 257
    frame #28: 0x00007ff8099985e7 HIToolbox`__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke_2 + 282
    frame #29: 0x00007ff80999839a HIToolbox`__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke + 274
    frame #30: 0x00007ff80998761a HIToolbox`TSMProcessRawKeyEventWithOptionsAndCompletionHandler + 3490
    frame #31: 0x00007ff8041218dd AppKit`__84-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:]_block_invoke_3.838 + 110
    frame #32: 0x00007ff804121637 AppKit`__204-[NSTextInputContext tryTSMProcessRawKeyEvent_orSubstitution:dispatchCondition:setupForDispatch:furtherCondition:doubleSpaceSubstitutionCondition:doubleSpaceSubstitutionWork:dispatchTSMWork:continuation:]_block_invoke.799 + 120
    frame #33: 0x00007ff80396d943 AppKit`-[NSTextInputContext tryTSMProcessRawKeyEvent_orSubstitution:dispatchCondition:setupForDispatch:furtherCondition:doubleSpaceSubstitutionCondition:doubleSpaceSubstitutionWork:dispatchTSMWork:continuation:] + 245
    frame #34: 0x00007ff80396d34f AppKit`-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:] + 1424
    frame #35: 0x00007ff80396cd84 AppKit`-[NSTextInputContext _handleEvent:allowingSyntheticEvent:] + 105
    frame #36: 0x00007ff80396cbfe AppKit`-[NSView interpretKeyEvents:] + 209
    frame #37: 0x0000000103e46d03 glfw-cocoa.so`-[GLFWContentView keyDown:] + 1523
    frame #38: 0x00007ff8038d5328 AppKit`-[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] + 7150
    frame #39: 0x00007ff8038d351a AppKit`-[NSWindow(NSEventRouting) sendEvent:] + 352
    frame #40: 0x00007ff8038d2356 AppKit`-[NSApplication(NSEvent) sendEvent:] + 3022
    frame #41: 0x00007ff803b8a89f AppKit`-[NSApplication _handleEvent:] + 65
    frame #42: 0x00007ff8037525ce AppKit`-[NSApplication run] + 623
...

That is no longer relevant to the responder chain.

... some methods were never called when the beep happened. ...

glfw/cocoa_window.m GLFWContentView doCommandBySelector never called

It looks like NSBeep first, then insert the text. Is there any useful information above? (I don't think so.)

Take ctrl+cmd+down as an example

// this will call insertText
[self interpretKeyEvents:[NSArray arrayWithObject:event]];

ctrl+cmd+down -> - (void)moveDown:(id)sender;
If moveDown is implemented, although there is no beep sound, InsertText will not be executed and the program in kitty will not receive key events.

This looks like a black box, poke around a bit and see what surprises pop up.

@kovidgoyal
Copy link
Owner

I'm somewhat confused. ctrl+cmd_down will not insert text anyway. So it doesnt matter if inserttext is called or not, as long as keyDown is called. So it looks like to fix this what we need is a list of command handler functions and just insert empty stubs for them.

@kovidgoyal
Copy link
Owner

Does this patch work:

diff --git a/glfw/cocoa_window.m b/glfw/cocoa_window.m
index fb5b2cc8..86b2330a 100644
--- a/glfw/cocoa_window.m
+++ b/glfw/cocoa_window.m
@@ -1461,6 +1461,13 @@ - (instancetype)initWithGlfwWindow:(NSRect)contentRect
     return self;
 }
 
+- (BOOL)performKeyEquivalent:(NSEvent *)event {
+    if (![[NSApp mainMenu] performKeyEquivalent:event]) {
+        [self.contentView keyDown:event];
+    }
+    return YES;
+}
+
 - (void) removeGLFWWindow
 {
     glfw_window = NULL;

@page-down
Copy link
Contributor

The above patch does not work, the shortcut still has sound.

Sorry, I was misled by the comments in the original code. You are right, these key combinations have nothing to do with insert text.

Does it mean that all modifier key combinations should not call interpretKeyEvents?

@kovidgoyal
Copy link
Owner

kovidgoyal commented Jan 10, 2022 via email

@page-down
Copy link
Contributor

Thanks for the explanation, it is indeed true that it needs to be sent unconditionally.

I'm also a little confused.

To sum up

  • Do not implement performKeyEquivalent to let each shortcut call individual command function.
  • Implement functions for each command, which cannot be empty stubs, to get the key events and send them. (Is it possible to get the key events in these functions?) e.g. moveDown -> send ctrl+cmd+down
  • All other keys are handled by keyDown.

Is this what you describe?

Here are the example cases

  • ctrl + a
    • keyDown -> no sound
    • doCommandBySelector: (moveToBeginningOfParagraph:)
    • no need to impl moveToBeginningOfParagraph
  • ctrl + cmd + down
    • keyDown -> interpretKeyEvents -> NSBeep
    • no doCommandBySelector, unknown command. learned from somewhere moveDown
    • with moveDown(), keyDown will not be called.
      • moveDown -> send ctrl+cmd+down to child program
      • (Will moveDown always be this combination? e.g. The user can modify the key binding in the system to call moveDown with ctrl+a.)
  • ctrl + /
    • keyDown -> interpretKeyEvents -> NSBeep
    • no doCommandBySelector, unknown command
    • unknown

I wonder if I misunderstood and if there is a better way to do this.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Jan 10, 2022 via email

@kovidgoyal
Copy link
Owner

If the commands are being called by something else, then we need to find out what that something is first, and then figure what to do based on that.

@kovidgoyal
Copy link
Owner

And looking at the call stack you posted, the something else is NSTextInputContent. The beep is coming from the system text input context (NSTextInputContext) in its doCommandBySelector method. There is no way of overriding that. So basically, there is no solution to this. As usual, cocoa is inadequate. So given that I think this is a CANTFIX, complain to Apple.

@page-down
Copy link
Contributor

Yes, if there is no way to control this behavior, then it is not fixable.

ctrl+cmd_down will not insert text anyway. So it doesnt matter if inserttext is called or not, as long as keyDown is called.

It looks like the only way to avoid this is to not send to NSTextInputContext. So when pressing ctrl+cmd+down, is it possible to set process_text to false and it doesn't seem to need insert text?

I did the following test and there is no sound anymore.

const bool process_text = (keycode != 0x7d /* down */ ) &&  ... ;

@kovidgoyal
Copy link
Owner

kovidgoyal commented Jan 10, 2022 via email

@page-down
Copy link
Contributor

page-down commented Jan 10, 2022

yes, but there is no way to know what key combinations will cause this.
Especially since users can override these in macos system preferences.

I have looked through the related issues from various open source projects and pressed almost all the key combinations.
For now, I have not found any other default combinations.

  • ctrl+cmd+down
  • ctrl+cmd+left
  • ctrl+cmd+right
  • ctrl+/
  • ctrl+kp_divide

The above basically covers all the cases where sound is present. I think it's good enough.
As for the user who have modified the key configuration, then they will have to suffer.

EDIT:

Then in order to avoid the beep you end up losing text.

I forgot that you can use "InsertText" in keybindings, which does lose text. You are right, it is not appropriate to do that.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Jan 10, 2022 via email

@michaelsbradleyjr
Copy link
Author

michaelsbradleyjr commented Jan 10, 2022

There is no way of overriding that. So basically, there is no solution to this. As usual, cocoa is inadequate. So given that I think this is a CANTFIX, complain to Apple.

Thanks for looking into this, @kovidgoyal and @page-down.

In its Profile -> Terminal preferences, iTerm2 has this:

image

And with those settings, here's what happens when I press ctrl+/ (5 times). There is no sound.

Screen.Recording.2022-01-10.at.10.26.21.mov

I realize iTerm2 and Kitty are completely different projects/programs, but it seems possible to intercept/silence it on macOS, at least in the cases of some key combos.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Jan 10, 2022 via email

@michaelsbradleyjr
Copy link
Author

am not willing to potentially break text input just to silence a bell

Fair enough.

Sucks for me since I'm really loving Kitty but the beeps for one of my all time most pressed key combos in Emacs (ctrl+/ := undo-tree-undo command) were/are driving me crazy. I'll map undo-tree-undo to another key combo and start fighting muscle memory.

In any case, Kitty is an awesome project – thank you for it and all your hard work on it!

@page-down
Copy link
Contributor

page-down commented Jan 10, 2022

@michaelsbradleyjr

~/Library/KeyBindings/DefaultKeyBinding.dict

{
    "@^\UF701" = "noop:";
    "@^\UF702" = "noop:";
    "@^\UF703" = "noop:";
    "^/" = "noop:";
}

Restart kitty and you will find no more sound.
It is worth noting that this will disable the default function of the key combinations.

@page-down
Copy link
Contributor

page-down commented Jan 10, 2022

@kovidgoyal

I take a second look at the other open source projects.

https://github.com/macvim-dev/macvim/blob/master/src/MacVim/MMAppController.m

See NSKeyBindingManager and comments

// Disable the default Cocoa "Key Bindings" since they interfere with the
// way Vim handles keyboard input.  Cocoa reads bindings from ...
...
// ... we load our own
// key binding dictionary from Resource/KeyBinding.plist.  We can't disable
// the bindings completely since it would break keyboard handling in ...
...
// It is possible to disable key bindings completely by not calling
// interpretKeyEvents: in keyDown: but this also disables key bindings used
// by certain input methods. ...
...

https://github.com/macvim-dev/macvim/blob/master/src/MacVim/KeyBinding.plist

Is this the right way to go?

michaelsbradleyjr added a commit to michaelsbradleyjr/prelude that referenced this issue Jan 10, 2022
See: kovidgoyal/kitty#4489 (comment)

For now can put a global workaround in
~/Library/KeyBindings/DefaultKeyBinding.dict

```
{
    "^/" = "noop:";
}
```
@kovidgoyal
Copy link
Owner

Isnt that basically the same as not calling interpretKeyEvents for
certain key combinations? The problem happens if the user remaps one of
those key combinations to do something meaningful or uses an IME that
uses one of them. We then dont follow the users preferences in either
case, thereby potentially breaking IME.

In that very macvim file for instance there is a mention of ctrl+spacke+-
being used by an IME. We dont have any guarantees that the keys we
override wont be needed.

@page-down
Copy link
Contributor

Yes, the problem is the inability to override individual commands and replace them with stubs, which is what you already pointed out.

Any other approaches that may cause text entry problems are not desirable.

I think that's the conclusion, at least for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants