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

protocols: add hyprland_lock_notify_v1 implementation #9092

Merged
merged 3 commits into from
Jan 19, 2025

Conversation

PaideiaDilemma
Copy link
Contributor

@PaideiaDilemma PaideiaDilemma commented Jan 16, 2025

Requires hyprwm/hyprland-protocols#12

Describe your PR, what does it fix/add?

Implements the proposed hyprland-lock-notify-v1 protocol.
It is basically IdleNotify, but no timer stuff. Pretty straight forward.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

No

Is it ready for merging, or does it need work?

Ready

@vaxerski
Copy link
Member

bump hyprland-protocols dep to 0.6.0 cuz ci fails

@fufexan
Copy link
Member

fufexan commented Jan 17, 2025

Meson too.

@PaideiaDilemma PaideiaDilemma force-pushed the main branch 3 times, most recently from ce00def to d43438d Compare January 17, 2025 16:36
@PaideiaDilemma PaideiaDilemma marked this pull request as ready for review January 17, 2025 17:09
src/protocols/LockNotify.cpp Outdated Show resolved Hide resolved
src/managers/ProtocolManager.cpp Show resolved Hide resolved
@PaideiaDilemma
Copy link
Contributor Author

PaideiaDilemma commented Jan 18, 2025

If we would relock the session with misc:allow_session_lock_restore, this would refuse to send locked again and log LOGM(ERR, "Not sending lock notification. Already locked!");

I think that behavior is fine, since something already went wrong if a user needs to restore and the error makes sense in that context. We wouldn't want to receive a second locked notification (would be a protocol error)

@vaxerski
Copy link
Member

yes thats fine

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

ret lgtm

src/protocols/LockNotify.cpp Outdated Show resolved Hide resolved
src/protocols/LockNotify.cpp Outdated Show resolved Hide resolved
src/protocols/LockNotify.cpp Outdated Show resolved Hide resolved
@PaideiaDilemma PaideiaDilemma force-pushed the main branch 3 times, most recently from 5c08bc2 to 1fb430b Compare January 19, 2025 12:04
Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

thanks

@vaxerski vaxerski merged commit 4074531 into hyprwm:main Jan 19, 2025
11 checks passed
littleblack111 pushed a commit to littleblack111/Hyprland that referenced this pull request Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants