-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Terminal doesn’t support audible bell (BEL, 0x7) #4046
Comments
I thought we had a duplicate for this, but it looks like we don’t. We had #2952, but that was tracking the pseudoconsole infrastructure for bell support. |
This will land after #780. |
Probably related to #2360 |
This commit moves the handling of the `BEL`, `BS`, `TAB`, and `CR` controls characters into the state machine (when in VT mode), instead of forwarding them on to the default string writer, which would otherwise have to parse them out all over again. This doesn't cover all the control characters, but `ESC`, `SUB`, and `CAN` are already an integral part of the `StateMachine` itself; `NUL` is filtered out by the `OutputStateMachineEngine`; and `LF`, `FF`, and `VT` are due to be implemented as part of PR #3271. Once all of these controls are handled at the state machine level, we can strip out all the VT-specific code from the `WriteCharsLegacy` function, which should simplify it considerably. This would also let us simplify the `Terminal::_WriteBuffer` implementation, and the planned replacement stream writer for issue #780. On the conhost side, the implementation is handled as follows: * The `BS` control is dispatched to the existing `CursorBackward` method, with a distance of 1. * The `TAB` control is dispatched to the existing `ForwardTab` method, with a tab count of 1. * The `CR` control required a new dispatch method, but the implementation was a simple call to the new `_CursorMovePosition` method from PR #3628. * The `BEL` control also required a new dispatch method, as well as an additional private API in the `ConGetSet` interface. But that's mostly boilerplate code - ultimately it just calls the `SendNotifyBeep` method. On the Windows Terminal side, not all dispatch methods are implemented. * There is an existing `CursorBackward` implementation, so `BS` works OK. * There isn't a `ForwardTab` implementation, but `TAB` isn't currently required by the conpty protocol. * I had to implement the `CarriageReturn` dispatch method, but that was a simple call to `Terminal::SetCursorPosition`. * The `WarningBell` method I've left unimplemented, because that functionality wasn't previously supported anyway, and there's an existing issue for that (#4046). ## Validation Steps Performed I've added a state machine test to confirm that the updated control characters are now forwarded to the appropriate dispatch handlers. But since the actual implementation is mostly relying on existing functionality, I'm assuming that code is already adequately tested elsewhere. That said, I have also run various manual tests of my own, and confirmed that everything still worked as well as before. References #3271 References #780 References #3628 References #4046
When is the BELL/Beep scheduled for? Initially it was planned for version 1.0.. or not? |
This commit makes the Windows Terminal play an audible sound when the `BEL` control character is output. The `BEL` control was already being forwarded through conpty, so it was just a matter of hooking up the `WarningBell` dispatch method to actually play a sound. I've used the `PlaySound` API to output the sound configured for the "Critical Stop" system event (aka _SystemHand_), since that is the sound used in conhost. ## Validation I've manually confirmed that the terminal produces the expected sound when executing `echo ^G` in a cmd shell, or `printf "\a"` in a WSL bash shell. References: * There is a separate issue (#1608) to deal with configuring the `BEL` to trigger visual forms of notification. * There is also an issue (#2360) requesting an option to disable the `BEL`. Closes #4046
🎉This issue was addressed in #7679, which has now been successfully released as Handy links: |
I'm having issue described in #6568 in v1.19.11213.0 Is the bell supposed to be playing Critical Stop sound? |
Environment
Microsoft Windows [Version 10.0.19030.1]
Windows Terminal 0.7.3451.0
Steps to reproduce
echo Ctrl+G
Expected behavior
A beep
Actual behavior
Silent
The text was updated successfully, but these errors were encountered: