-
Notifications
You must be signed in to change notification settings - Fork 4
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 i3/Sway support #8
Conversation
Looks largely copied from hyprland ipc at this point so I figured I'd mention a few things:
|
Thanks for the feedback, I wanted to stay mainly in line with the existing code, but I do have some questions:
|
Also make sure you have a way to turn a sway monitor into a qs monitor. |
c061ef6
to
e899eb4
Compare
4a93776
to
30aa969
Compare
You've force pushed a lot, this ready for review? |
Yep, you could review it, I would still want to implement more i3/Sway IPC features, but I think that will be for a later PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bunch of nitpicks. I assume you've gone through and tested everything manually as well, run the lints, etc. If you have a tester file send that along as well and I'll give it a try before merging.
I have just noticed a crash related to those changes, don't know how to replicate it yet but I would need to fix that before this PR can progress edit: fixed, was a lot more straightforward than I thought |
Consider using QDataStream transactions instead. As is, if you break halfway through an event then the following event will be invalid and so on. check ipc.cpp for an example |
I ended up doing that with QByteArrays instead, if I encounter a broken message i try to find the next occurrence of the magic sequence and skip to there. However this is under the assumption that the Sway IPC does not write partial messages to the socket, and it does not seem to do so. |
Do we ever encounter broken messages? i3 shouldn't be sending those at all. I think the best solution to read the header would be a QDataStream transaction, which lets you do a partial read pretty easily. Basically: stream.startTransaction()
stream.startTransaction()
stream >> magic >> header bits >> size;
if (!stream.commitTransaction()) return;
stream >> data; // assuming you need size to find data. nested commitTransaction works as a barrier for the integrity of values read previosuly
if (!stream.commitTransaction()) return; You can nest transactions and it works pretty well. This should be more reliable than skipping events. Failing commits roll back the stream, and if there's enough data on the next readyread it should work. |
de9da8d
to
35191b1
Compare
Alrighty, that should be implemented now, here is also a file to test the IPC impl: Details
import QtQuick
import Quickshell
import Quickshell.I3
ShellRoot {
PanelWindow {
id: panel
Timer {
id: timer
}
Component.onCompleted: {
const program = "foot"; // change to an installed program
const longLog = false; // toggle logging of workspace & monitor objects
const stringify = v => JSON.stringify(v, null, 2);
console.log("Connecting to", I3.socketPath);
I3.rawEvent.connect(e => {
console.log("Event:", e.type);
});
I3.focusedWorkspaceChanged.connect(() => {
console.log("Changed to workspace", I3.focusedWorkspace.name, I3.focusedMonitor.focusedWorkspace.name);
console.log("Focused monitor's focused workspace and actual focused workspace should be the same:");
console.log(`${I3.focusedMonitor.focusedWorkspace.name} and ${I3.focusedWorkspace.name}`);
});
I3.focusedMonitorChanged.connect(() => {
console.log("Changed to monitor", I3.focusedMonitor.name);
});
for (const monitor of I3.monitors.values) {
console.log("Monitor:", monitor.name);
longLog && console.log(stringify(monitor));
}
for (const workspace of I3.workspaces.values) {
console.log("Workspace:", workspace.name);
longLog && console.log(stringify(workspace));
I3.dispatch(`workspace ${workspace.num}`);
}
I3.dispatch(`workspace 13`);
console.log(`Changing workspace response: ${stringify(I3.dispatch(`exec ${program}`))}`);
console.log(`Bad request response: ${stringify(I3.dispatch(`quickshell`))}`);
I3.dispatch(`workspace 1`);
I3.refreshWorkspaces();
I3.refreshMonitors();
console.log(stringify(I3.monitorFor(panel.screen)));
}
}
} |
Until you commit the transaction you shouldn't access data that is part of it, as it might not have been read. Also, rolling back the transaction to read again when you hit something invalid will just hang. On my phone rn, will do a proper code review again later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might've missed things again, but this is what I saw this time. Apologies for the repeated re-reviews but I want to be sure quickshell's integrations are stable.
Should be all fixed now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't see anything else at a glance, will give it a more thorough lookover again shortly.
We shouldn't be trying to write to the socket unless we're actually connected. I think hyprland IPC also does this though, now that I think about it. Will fix that later.
|
Failing lints
|
Fixed |
449327b
to
2571766
Compare
sway: add urgent and focused dispatchers to workspaces flake: add sway toggle WIP sway: add monitor status sway: handle multiple ipc events in one line sway: reuse socket connection for dispatches & better command type handling WIP sway: add associated monitor to a workspace i3/sway: update to allow for i3 compatibility i3/sway: manage setting the focused monitors i3/sway: fix multi monitor crash i3/sway: fix linting errors i3/sway: update nix package flag naming to i3 i3/sway: add documentation, fix module.md and impl monitorFor i3/sway: handle more workspace ipc events i3/sway: fix review i3/sway: fix crash due to newline breaking up an IPC message i3/sway: handle broken messages by forwarding to the next magic sequence i3/sway: break loop when buffer is empty i3/sway: fix monitor focus & focused monitor signal not being emitted i3/sway: use datastreams instead of qbytearrays for socket reading i3/sway: fix lint issues i3/sway: drop second socket connection, remove dispatch return value, recreate IPC connection on fatal error i3/sway: handle run_command responses i3/sway: remove reconnection on unknown event i3/sway: fix formatting, lint & avoid writing to socket if connection is not open
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This pr adds support for retrieving data from Sway's IPC.
Features:
Additional Todo: