Conversation
- Consolidate planar::Window with converters::Window into types::Window - Move planar::Rect into types - Replace to_window with From<&Value> for Window
i3 window manager doesn't report proper positions for contenarized windows, so we have to translate the positions to root window. As far as visibility goes, some i3 builds (arch i3-gaps) dont show window hints as expected (_NET_WM_STATE_HIDDEN, _NET_WM_STATE_MAXIMIZED_VERT, _NET_WM_STATE_MAXIMIZED_HORZ), this makes distinction through EWMH for visible windows impossible. Instead we use WM_STATE_WITHDRAWN, to distinguish between hidden windows and not. Known-Bugs: Floating state is not properly detected.
Previous fetching by type for each window, didn't guarantee the order of the replies, because each reply was of the same fingerprint. Now we request different properties per window which solved issues with incorrect data for each window. It also cleaned up the API a bunch.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces two new backends (xcb and wmctl), replaces clap with a custom CLI parser for faster startup, and refactors core modules to decouple types and remove JSON-specific logic.
- Add xcb and wmctl backends behind feature flags and update dependency installation.
- Refactor navigation and main logic to use a
Backendtrait instead of direct JSON parsing. - Clean up converter code and centralize
Window/Recttypes intypesmodule.
Reviewed Changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/install_deps.sh | Install libxcb1 packages for the new xcb backend |
| rust/src/types/window.rs & mod.rs | Define shared Window struct and Windows alias |
| rust/src/planar/* | Switch to using centralized types::Rect and Window |
| rust/src/navigation.rs | Update navigation to use generic GetVisible/GetTabs |
| rust/src/cli.rs | Replace clap with custom CLI parsing (UseBackend) |
| rust/src/main.rs | Wire up backend selection and remove clap dependency |
| rust/src/backend/* | Introduce traits and implementations for i3, xcb, wmctl |
| rust/Cargo.toml.in | Drop clap, add optional deps and feature definitions |
| rust/Makefile | Run tests with --all-features |
| rust/README.md | Document build features and available backends |
Comments suppressed due to low confidence (1)
rust/src/cli.rs:12
- [nitpick] The CLI uses
UseBackendwhile the backend module usesUsedBackend, which is easy to confuse. Rename one of these enums for consistency (e.g., unify both toUsedBackend).
pub struct Cli {
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Additional backends using
xcbandwmctlallow to use some functionalities faster and on different window managers than i3. Although they have no way of distinguishing between floating windows or not.Currently xcb is up to 5x faster than i3 implementation.
Option to select a backend was added with
-i3,-wmand-xcbflag. Although to implement that I had to re-work the cli (which now being home-grown solution should also be quite a bit faster). Clap dependency has been dropped.I like that while working on the solution I managed to decouple quite a bit of code, this increased clarity and allowed use of features to select which backends we want to use.
Known Bugs: wmctl backends fails on i3 with missing _NET_DESKTOP hint. So while it should work, it's unknown if it does, so it's not a default feature.