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

Add window focused signal #274

Merged
merged 2 commits into from
Jan 29, 2025
Merged

Add window focused signal #274

merged 2 commits into from
Jan 29, 2025

Conversation

anriha
Copy link
Contributor

@anriha anriha commented Jan 29, 2025

This PR adds a new signal that gets triggered whenever a window receives keyboard focus. I've tested the core functionality and it works as expected, though I haven't tested the Lua API yet (it's a bit tricky to set up on NixOS).

This is just the first of several signals I'm planning to add. My goal is to implement enough signals to replicate the dwl format for yambar integration, as I'd like to make pinnacle my daily driver.

Personal note: I'm really excited about this project! I've tried many Wayland compositors, and was actually planning to write my own because each existing one was missing something crucial. Then I found pinnacle, and it's exactly what I was looking for. Really happy to be able to contribute.

Copy link
Collaborator

@Ottatop Ottatop left a comment

Choose a reason for hiding this comment

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

Oh hey I was thinking about this a few days ago

Implementation looks good! Did a sanity check and it does work on the Lua side. Just a minor comment nit.

I was considering whether or not we could extend this to something like fn(Option<&WindowHandle>, Option<&WindowHandle>) and give people both the old and new focuses as well as notifying on focus loss, but I'm not sure how to handle the case where a window gets closed. In that case I don't really want to pass a dead window handle to the signal because it's not useful. We can roll with this until something gets figured out.

api/rust/src/signal.rs Outdated Show resolved Hide resolved
Co-authored-by: Ottatop <120758733+Ottatop@users.noreply.github.com>
@anriha
Copy link
Contributor Author

anriha commented Jan 29, 2025

Yeah, I thought about that possibility too. Definitely more edge cases to consider there, especially with the closed window scenario you mentioned. And honestly, I suspect the use case for having both old/new focus info would be pretty rare in practice. But I'll keep it in mind while I'm working on other signals - might stumble upon a clean solution along the way.

@Ottatop
Copy link
Collaborator

Ottatop commented Jan 29, 2025

Aight lgtm, thank you!

@Ottatop Ottatop merged commit a3afd9e into pinnacle-comp:main Jan 29, 2025
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.

2 participants