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

gtk: terminal bell #2231

Closed
wants to merge 4 commits into from
Closed

gtk: terminal bell #2231

wants to merge 4 commits into from

Conversation

jcollie
Copy link
Collaborator

@jcollie jcollie commented Sep 12, 2024

Use GTK's media streaming APIs to implement the terminal bell. By default it's disabled since it's likely that only a small subset of people will want this feature. Currently only audio support is implemented but there's scaffolding for several other methods of alerting the user.

NOTE: GTK needs to have access to GStreamer and the "base" and "good" plugins or it crashes when you try to play a sound. I've added the necessary packages to the NixOS developer shell and package but I haven't found a method yet to detect the situation and avoid the crash.

@jcollie
Copy link
Collaborator Author

jcollie commented Sep 13, 2024

Added @pluiedev 's patches for x11/wayland attention.

@jcollie
Copy link
Collaborator Author

jcollie commented Sep 15, 2024

rebased onto current main to pick up the desktop notification fixes from #2163

src/config/Config.zig Outdated Show resolved Hide resolved
@jcollie
Copy link
Collaborator Author

jcollie commented Sep 16, 2024

Rebased onto current main to fix conflict caused by Toast PR.

mitchellh added a commit that referenced this pull request Sep 16, 2024
The syntax of tagged unions is `tag:value`. This matches the tagged
union parsing syntax for keybindings (i.e. `new_split:right`).

I'm adding this now on its own without a user-facing feature because
I can see some places we might use this and I want to separate this out.
There is already a PR open now that can utilize this (#2231).
mitchellh added a commit that referenced this pull request Sep 16, 2024
The syntax of tagged unions is `tag:value`. This matches the tagged
union parsing syntax for keybindings (i.e. `new_split:right`).

I'm adding this now on its own without a user-facing feature because
I can see some places we might use this and I want to separate this out.
There is already a PR open now that can utilize this (#2231).
mitchellh added a commit that referenced this pull request Sep 16, 2024
The syntax of tagged unions is `tag:value`. This matches the tagged
union parsing syntax for keybindings (i.e. `new_split:right`).

I'm adding this now on its own without a user-facing feature because
I can see some places we might use this and I want to separate this out.
There is already a PR open now that can utilize this (#2231).
mitchellh added a commit that referenced this pull request Sep 16, 2024
The syntax of tagged unions is `tag:value`. This matches the tagged
union parsing syntax for keybindings (i.e. `new_split:right`).

I'm adding this now on its own without a user-facing feature because
I can see some places we might use this and I want to separate this out.
There is already a PR open now that can utilize this (#2231).
mitchellh added a commit that referenced this pull request Sep 16, 2024
The syntax of tagged unions is `tag:value`. This matches the tagged
union parsing syntax for keybindings (i.e. `new_split:right`).

I'm adding this now on its own without a user-facing feature because
I can see some places we might use this and I want to separate this out.
There is already a PR open now that can utilize this (#2231).
@jcollie
Copy link
Collaborator Author

jcollie commented Sep 17, 2024

Rebased on to current main and adjusted the code to take advantage of the generic tagged union parsing.

Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my biggest issue is the complexity Wayland introduces.

A few requests:

  • Can we remove Wayland from this PR?
  • In the future, can we use the Wayland C API rather than Zig? That'll make it easier for us to upgrade Zig versions?

build.zig.zon Outdated Show resolved Hide resolved
@jcollie
Copy link
Collaborator Author

jcollie commented Sep 18, 2024

I've backed out the Wayland patches.

@caiodelgadonew
Copy link

This is a great feature! Thanks!

@jcollie
Copy link
Collaborator Author

jcollie commented Sep 26, 2024

rebased onto current main

@jcollie
Copy link
Collaborator Author

jcollie commented Oct 19, 2024

rebased to latest main

Use GTK's media streaming APIs to implement the terminal bell. By
default it's disabled since it's likely that only a small subset
of people will want this feature. Currently only audio support is
implemented but there's scaffolding for several other methods of
alerting the user.

NOTE: GTK needs to have access to GStreamer and the "base" and "good"
plugins or it crashes when you try to play a sound. I've added the
necessary packages to the NixOS developer shell and package but I
haven't found a method yet to detect the situation and avoid the crash.
@jcollie jcollie closed this by deleting the head repository Dec 27, 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