Add integrated terminal bell + settings - #53752
Conversation
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
This should allow for potential future enhancements like visual bell, custom sounds, etc. if that becomes desirable (e.g. like ghostty)
| "show_count_badge": false, | ||
| "bell": { | ||
| // Whether to invoke the OS-default alert sound when the terminal bell (BEL character) is printed. | ||
| "system": true, |
There was a problem hiding this comment.
This settings structure doesn't really make sense as it is.
If we were to add different sound options the settings would look like
"bell": {
"system": false,
"other-sound": true,
}This should be
"bell": "off" // default
"bell": "system" // use system alert soundor similar
There was a problem hiding this comment.
That's fair, I was thinking a struct/object made sense to allow for the possibility of visual bells or other indicators (like desktop notifications, toasts, whatever), as suggested in #47531 (comment)
Also was looking at VSCode for inspiration here, they have an accessibility setting for announcing the terminal bell (probably via some text-to-speech or something): https://github.com/microsoft/vscode/blob/1faf9f4d1bab92b16b16cc6306f828a8f32567bf/src/vs/workbench/contrib/accessibility/browser/accessibilityConfiguration.ts#L497-L510
What do you think about something like this?
"bell": {
"audio": "system", // or "off", etc.
// possible future enhancements:
"visual": "off",
"notification": "on",
"announce": "off"
}There was a problem hiding this comment.
Interesting. I think we should keep it simple, with no nested object for now. It's fairly easy to do a settings migration and add the extra structure if we ever add more capabilities. Until then it's just extra typing in the settings JSON, making the setting less discoverable.
| pub show_count_badge: Option<bool>, | ||
| /// What to do when the `BEL` character (`\a`) is printed to terminal. | ||
| /// | ||
| /// Default: "system" |
There was a problem hiding this comment.
This was missed when changing the default.
Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] ~Unsafe blocks (if any) have justifying comments~ - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Followup to zed-industries#47531 to use the gpui feature in Zed. This just plumbs the "system bell" feature into the terminal, behind a new setting (enabled by default, like most other terminal applications). Closes zed-industries#5303 Relates to zed-industries#40826 (comment) Release Notes: - Added audible BEL to Terminal; can be enabled by setting `terminal.bell` to `"system"`. --------- Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Ben Kunkle <Ben.kunkle@gmail.com>
Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] ~Unsafe blocks (if any) have justifying comments~ - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Followup to zed-industries#47531 to use the gpui feature in Zed. This just plumbs the "system bell" feature into the terminal, behind a new setting (enabled by default, like most other terminal applications). Closes zed-industries#5303 Relates to zed-industries#40826 (comment) Release Notes: - Added audible BEL to Terminal; can be enabled by setting `terminal.bell` to `"system"`. --------- Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Ben Kunkle <Ben.kunkle@gmail.com>
Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] ~Unsafe blocks (if any) have justifying comments~ - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Followup to zed-industries#47531 to use the gpui feature in Zed. This just plumbs the "system bell" feature into the terminal, behind a new setting (enabled by default, like most other terminal applications). Closes zed-industries#5303 Relates to zed-industries#40826 (comment) Release Notes: - Added audible BEL to Terminal; can be enabled by setting `terminal.bell` to `"system"`. --------- Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Ben Kunkle <Ben.kunkle@gmail.com>
Self-Review Checklist:
Unsafe blocks (if any) have justifying commentsFollowup to #47531 to use the gpui feature in Zed. This just plumbs the "system bell" feature into the terminal, behind a new setting (enabled by default, like most other terminal applications).
Closes #5303
Relates to #40826 (comment)
Release Notes:
terminal.bellto"system".