-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add a setting to configure the audible bell #7793
Conversation
@@ -76,6 +76,7 @@ namespace winrt::TerminalApp::implementation | |||
GETSET_PROPERTY(bool, StartOnUserLogin, false); | |||
GETSET_PROPERTY(bool, AlwaysOnTop, false); | |||
GETSET_PROPERTY(bool, UseTabSwitcher, true); | |||
GETSET_PROPERTY(winrt::TerminalApp::BellStyle, BellStyle, winrt::TerminalApp::BellStyle::Audible); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 Carlos, this will be a conflict for you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect. Do we want it to be per-profile?
This will have an interesting effect on some applications, and users may be confused by it. There's some applications that emit their own bells using the same API, and people may be upset that we can't suppress them.
Not a complaint, just something to be aware of!
This could eventually be a flag mapper ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted this the moment I saw #7679
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the docs and schema!
Also, please make my life easier and wait until after #7667 merges.
…isable-bell # Conflicts: # src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl
…isable-bell # Conflicts: # src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp # src/cascadia/TerminalSettingsModel/GlobalAppSettings.h # src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl # src/cascadia/TerminalSettingsModel/defaults.json
I updated the docs and schema
"bellStyle": { | ||
"default": "audible", | ||
"description": "Controls what happens when the application emits a BEL character. When set to \"audible\", the Terminal will play a sound. When set to \"none\", nothing will happen.", | ||
"$ref": "#/definitions/BellStyle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm glad it caught this b/c it wouldn't have been a valid schema otherwise!@ 😉
## Summary of the Pull Request Since we've started working on the Settings UI, a few settings have been added on `main`. This adds those missing settings over. ## References Missing settings include... - #7364: `disableAnimations` - #7873: `launchMode` `focus` and `maximizedFocus` - #7793: `bellStyle` ## Validation Steps Performed Verified that those settings appear properly in the Settings UI.
🎉 Handy links: |
Summary of the Pull Request
Adds a new setting,
bellStyle
, to be able to disable the audible bell added in #7679. Currently, this setting accepts two values:audible
: play a noise on a bellnone
: Don't play a noise.In the future, we can add a
"bellStyle": "visible"
for flashing the Terminal instead of making a noise on bell.References
PR Checklist
Detailed Description of the Pull Request / Additional comments
FOR DISCUSSION: Should
bellStyle
just bebell
?Validation Steps Performed
Pressing Ctrl+G in cmd, and hitting enter is an easy way of triggering a bell. I set the setting to
none
, and presto, the bell stopped.