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

feat: add wm-toggle-pause command #828

Merged
merged 11 commits into from
Nov 30, 2024
Merged

feat: add wm-toggle-pause command #828

merged 11 commits into from
Nov 30, 2024

Conversation

michidk
Copy link
Contributor

@michidk michidk commented Nov 3, 2024

This is my try to implement the pause command I proposed in #825.

Basically, this adds the command wm-toggle-pause which pauses glazewm from managing windows.

Feedback is very welcome. This is my first contribution to glazewm and I'm not too familiar with the codebase yet.

@lars-berger
Copy link
Member

Ay this is super cool 👌 Thanks for putting together the IPC side of it as well.

A downside with returning early in all these platform events is that state will be out of sync on resume. Like say a monitor was unplugged, the WM would still think it has all its monitors.

Perhaps it'd be better to instead skip redraws if the WM is paused:

// wm.rs - https://github.com/glzr-io/glazewm/blob/940bb4cc474898aa3b1ffda2f29720e37bb7a16c/packages/wm/src/wm.rs#L128
..
    if state.is_paused {
      platform_sync(state, config)?;
    }

and then also disable keyboard/mouse listener on pause:

// main.rs - https://github.com/glzr-io/glazewm/blob/940bb4cc474898aa3b1ffda2f29720e37bb7a16c/packages/wm/src/main.rs#L162
..
        if matches!(
          wm_event,
          WmEvent::UserConfigChanged { .. }
            | WmEvent::BindingModesChanged { .. }
            | WmEvent::PauseChanged { .. }
        ) {
          event_listener.update(
            &config,
            &wm.state.binding_modes,
            is_paused
          );
        }

@michidk
Copy link
Contributor Author

michidk commented Nov 17, 2024

Hi @lars-berger,

My new implementation to block mouse events goes probably into the right direction.

But there is one problem with blocking key events: The keybinding to unpause won't be recognized as well.

However, I went forward and implemented something:
If you think this is an alright approach, I would go and implement the last missing part:
https://github.com/michidk/glazewm/blob/pause/packages/wm/src/common/platform/keyboard_hook.rs#L365
(detecting if the keybind is for unpausing and then not blocking it).

@michidk michidk marked this pull request as ready for review November 28, 2024 11:27
@michidk
Copy link
Contributor Author

michidk commented Nov 28, 2024

I finished & tested the implementation!
Next, I'm going to start working on the necessary changes in zebar for displaying the paused state.

@michidk
Copy link
Contributor Author

michidk commented Nov 28, 2024

Done.

glazewm-js: glzr-io/glazewm-js#15
zebar: glzr-io/zebar#167

@lars-berger lars-berger changed the title feat: Add pause command (closes #825) feat: add wm-toggle-pause command Nov 30, 2024
@lars-berger lars-berger merged commit 3429eec into glzr-io:main Nov 30, 2024
2 checks passed
@lars-berger
Copy link
Member

All merged now! Thanks for getting it all nicely integrated with the NPM library + zebar as well 🙌

Found a couple minor things while testing it out more thoroughly #870. Also did a super nitpicky rename to is_paused since that seems more consistent with our other boolean properties like has_focus, is_displayed...

Thanks again for the massive help on this ❤️

Copy link

github-actions bot commented Dec 3, 2024

🎉 This PR is included in version 3.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@michidk
Copy link
Contributor Author

michidk commented Dec 7, 2024

I actually had used is_paused in some iterations as well, but then removed it again.

I think right now we sometimes have still paused in the NPM library but I guess that's fine.

Thank you for finishing this up and sorry for missing the resize & IPC events!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants