Skip to content

Commit e946ef9

Browse files
committed
i3/sway: fix review
1 parent d54fe49 commit e946ef9

File tree

2 files changed

+44
-46
lines changed

2 files changed

+44
-46
lines changed

src/x11/i3/ipc/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ qt_add_library(quickshell-i3-ipc STATIC
88
qt_add_qml_module(quickshell-i3-ipc
99
URI Quickshell.I3._Ipc
1010
VERSION 0.1
11-
DEPENDENCIES QtQml Quickshell
11+
DEPENDENCIES QtQml
1212
)
1313

1414
qs_add_module_deps_light(quickshell-i3-ipc Quickshell)

src/x11/i3/ipc/connection.cpp

Lines changed: 43 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#include <algorithm>
2-
#include <cstring>
32
#include <tuple>
43

4+
#include <bit>
55
#include <qbytearray.h>
66
#include <qbytearrayview.h>
77
#include <qcontainerfwd.h>
@@ -25,8 +25,8 @@
2525
#include "monitor.hpp"
2626
#include "workspace.hpp"
2727

28-
Q_LOGGING_CATEGORY(logi3Ipc, "quickshell.I3.ipc", QtWarningMsg);
29-
Q_LOGGING_CATEGORY(logi3IpcEvents, "quickshell.I3.ipc.events", QtWarningMsg);
28+
Q_LOGGING_CATEGORY(logI3Ipc, "quickshell.I3.ipc", QtWarningMsg);
29+
Q_LOGGING_CATEGORY(logI3IpcEvents, "quickshell.I3.ipc.events", QtWarningMsg);
3030

3131
namespace qs::i3::ipc {
3232

@@ -38,7 +38,7 @@ QVector<Event> I3Ipc::makeRequest(const QByteArray& request) {
3838
auto dataRaw = this->commandSocket.readAll();
3939
return I3Ipc::parseResponse(dataRaw);
4040
} else {
41-
qWarning(logi3Ipc) << "Timeout while waiting for read for" << request;
41+
qCWarning(logI3Ipc) << "Timeout while waiting for read for" << request;
4242
return {};
4343
}
4444
}
@@ -49,7 +49,7 @@ void I3Ipc::dispatch(const QString& payload) {
4949
QVector<Event> res = this->makeRequest(message);
5050

5151
if (res.isEmpty()) {
52-
qWarning() << "IPC did not reply, error while executing" << payload;
52+
qCWarning(logI3Ipc) << "IPC did not reply, error while executing" << payload;
5353
}
5454

5555
auto [_, data] = res.first();
@@ -59,35 +59,33 @@ void I3Ipc::dispatch(const QString& payload) {
5959
const bool success = jsonMessage["success"].toBool();
6060

6161
if (!success) {
62-
qWarning() << "Error while executing dispatch of" << payload << ":\n\t"
63-
<< jsonMessage["error"];
62+
qCWarning(logI3Ipc) << "Error while executing dispatch of" << payload << ":\n\t"
63+
<< jsonMessage["error"];
6464
}
6565
}
6666
}
6767

6868
QByteArray I3Ipc::buildRequestMessage(EventCode cmd, const QByteArray& payload) {
69-
int len = static_cast<int>(payload.length());
70-
QByteArray lenBytes = QByteArray(4, Qt::Initialization::Uninitialized);
71-
QByteArray typeBytes = QByteArray(4, Qt::Initialization::Uninitialized);
69+
auto payloadLength = static_cast<quint32>(payload.length());
7270

73-
std::memcpy(lenBytes.data(), &len, 4);
74-
std::memcpy(typeBytes.data(), &cmd, 4);
71+
auto type = QByteArray(std::bit_cast<std::array<char, 4>>(cmd).data(), 4);
72+
auto len = QByteArray(std::bit_cast<std::array<char, 4>>(payloadLength).data(), 4);
7573

76-
QByteArray data = QByteArray(MAGIC.data()) + lenBytes + typeBytes + payload;
74+
qCWarning(logI3Ipc) << MAGIC.data() + len + type + payload;
7775

78-
return data;
76+
return MAGIC.data() + len + type + payload;
7977
}
8078

8179
I3Ipc::I3Ipc() {
8280
auto sock = qEnvironmentVariable("I3SOCK");
8381

8482
if (sock.isEmpty()) {
85-
qWarning() << "$I3SOCK is unset. Trying $SWAYSOCK Cannot connect to I3.";
83+
qCWarning(logI3Ipc) << "$I3SOCK is unset. Trying $SWAYSOCK.";
8684

8785
sock = qEnvironmentVariable("SWAYSOCK");
8886

8987
if (sock.isEmpty()) {
90-
qWarning() << "$SWAYSOCK and I3SOCK are unset. Cannot connect to socket.";
88+
qCWarning(logI3Ipc) << "$SWAYSOCK and I3SOCK are unset. Cannot connect to socket.";
9189
return;
9290
}
9391
}
@@ -154,7 +152,7 @@ QVector<Event> I3Ipc::parseResponse(QByteArray rawEvent) {
154152
ds >> type;
155153

156154
if (I3IpcEvent::intToEvent(type) == EventCode::UNKNOWN) {
157-
qWarning(logi3Ipc) << "Received unknown event" << rawEvent;
155+
qCWarning(logI3Ipc) << "Received unknown event" << rawEvent;
158156
return events;
159157
}
160158

@@ -165,7 +163,7 @@ QVector<Event> I3Ipc::parseResponse(QByteArray rawEvent) {
165163
auto data = QJsonDocument::fromJson(byteData, &e);
166164

167165
if (e.error != QJsonParseError::NoError) {
168-
qWarning(logi3Ipc) << "Invalid JSON value:" << e.errorString() << "\n\t" << byteData;
166+
qCWarning(logI3Ipc) << "Invalid JSON value:" << e.errorString() << "\n\t" << byteData;
169167
} else {
170168
events.push_back(std::tuple(I3IpcEvent::intToEvent(type), data));
171169
}
@@ -178,18 +176,18 @@ QVector<Event> I3Ipc::parseResponse(QByteArray rawEvent) {
178176

179177
void I3Ipc::eventSocketError(QLocalSocket::LocalSocketError error) const {
180178
if (!this->valid) {
181-
qWarning() << "Unable to connect to I3 socket:" << error;
179+
qCWarning(logI3Ipc) << "Unable to connect to I3 socket:" << error;
182180
} else {
183-
qWarning() << "I3 socket error:" << error;
181+
qCWarning(logI3Ipc) << "I3 socket error:" << error;
184182
}
185183
}
186184

187185
void I3Ipc::eventSocketStateChanged(QLocalSocket::LocalSocketState state) {
188186
if (state == QLocalSocket::ConnectedState) {
189-
qCInfo(logi3Ipc) << "I3 event socket connected.";
187+
qCInfo(logI3Ipc) << "I3 event socket connected.";
190188
emit this->connected();
191189
} else if (state == QLocalSocket::UnconnectedState && this->valid) {
192-
qCWarning(logi3Ipc) << "I3 event socket disconnected.";
190+
qCWarning(logI3Ipc) << "I3 event socket disconnected.";
193191
}
194192

195193
this->valid = state == QLocalSocket::ConnectedState;
@@ -255,7 +253,7 @@ void I3Ipc::refreshWorkspaces() {
255253
auto res = this->makeRequest(msg);
256254

257255
if (res.isEmpty()) {
258-
qWarning(logi3Ipc) << "Failed updating workspaces";
256+
qCWarning(logI3Ipc) << "Failed updating workspaces";
259257
return;
260258
}
261259

@@ -266,7 +264,7 @@ void I3Ipc::refreshWorkspaces() {
266264
const auto& mList = this->mWorkspaces.valueList();
267265
auto names = QVector<QString>();
268266

269-
qInfo(logi3Ipc) << "There are" << workspaces.toVariantList().length() << "workspaces";
267+
qCDebug(logI3Ipc) << "There are" << workspaces.toVariantList().length() << "workspaces";
270268
for (auto entry: workspaces) {
271269
auto object = entry.toObject().toVariantMap();
272270
auto name = object["name"].toString();
@@ -303,7 +301,7 @@ void I3Ipc::refreshWorkspaces() {
303301
}
304302
}
305303

306-
qInfo(logi3Ipc) << "Removing" << removedWorkspaces.length() << "deleted workspaces.";
304+
qCDebug(logI3Ipc) << "Removing" << removedWorkspaces.length() << "deleted workspaces.";
307305

308306
for (auto* workspace: removedWorkspaces) {
309307
this->mWorkspaces.removeObject(workspace);
@@ -316,7 +314,7 @@ void I3Ipc::refreshMonitors() {
316314
auto res = this->makeRequest(msg);
317315

318316
if (res.isEmpty()) {
319-
qWarning(logi3Ipc) << "Failed to update monitors";
317+
qCWarning(logI3Ipc) << "Failed to update monitors";
320318
return;
321319
}
322320

@@ -326,7 +324,7 @@ void I3Ipc::refreshMonitors() {
326324
const auto& mList = this->mMonitors.valueList();
327325
auto names = QVector<QString>();
328326

329-
qInfo(logi3Ipc) << "There are" << monitors.toVariantList().length() << "monitors";
327+
qCDebug(logI3Ipc) << "There are" << monitors.toVariantList().length() << "monitors";
330328

331329
for (auto elem: monitors) {
332330
auto object = elem.toObject().toVariantMap();
@@ -364,7 +362,7 @@ void I3Ipc::refreshMonitors() {
364362
}
365363
}
366364

367-
qInfo(logi3Ipc) << "Removing" << removedMonitors.length() << "disconnected monitors.";
365+
qCDebug(logI3Ipc) << "Removing" << removedMonitors.length() << "disconnected monitors.";
368366

369367
for (auto* monitor: removedMonitors) {
370368
this->mMonitors.removeObject(monitor);
@@ -377,14 +375,14 @@ void I3Ipc::onEvent(I3IpcEvent* event) {
377375
case EventCode::WORKSPACE: this->handleWorkspaceEvent(event); return;
378376
case EventCode::OUTPUT:
379377
/// I3 only sends an "unspecified" event, so we have to query the data changes ourselves
380-
qInfo(logi3Ipc) << "Refreshing Monitors...";
378+
qCInfo(logI3Ipc) << "Refreshing Monitors...";
381379
this->refreshMonitors();
382380
return;
383-
case EventCode::SUBSCRIBE: qInfo(logi3Ipc) << "Connected to IPC"; return;
381+
case EventCode::SUBSCRIBE: qCInfo(logI3Ipc) << "Connected to IPC"; return;
384382
case EventCode::UNKNOWN:
385-
qWarning(logi3Ipc) << "Unknown event:" << event->type() << event->data();
383+
qCWarning(logI3Ipc) << "Unknown event:" << event->type() << event->data();
386384
return;
387-
default: qWarning(logi3Ipc) << "Unhandled event:" << event->type();
385+
default: qCWarning(logI3Ipc) << "Unhandled event:" << event->type();
388386
}
389387
}
390388

@@ -394,7 +392,7 @@ void I3Ipc::handleWorkspaceEvent(I3IpcEvent* event) {
394392
auto change = event->mData["change"];
395393

396394
if (change == "init") {
397-
qCInfo(logi3IpcEvents) << "New workspace has been created";
395+
qCInfo(logI3IpcEvents) << "New workspace has been created";
398396

399397
auto workspaceData = event->mData["current"];
400398

@@ -405,13 +403,13 @@ void I3Ipc::handleWorkspaceEvent(I3IpcEvent* event) {
405403
}
406404

407405
this->mWorkspaces.insertObject(workspace);
408-
qCInfo(logi3Ipc) << "Added workspace to list";
406+
qCInfo(logI3Ipc) << "Added workspace to list";
409407

410408
} else if (change == "focus") {
411409
auto oldName = event->mData["old"]["name"].toString();
412410
auto newName = event->mData["current"]["name"].toString();
413411

414-
qCInfo(logi3IpcEvents) << "Focus changed: " << oldName << "->" << newName;
412+
qCInfo(logI3IpcEvents) << "Focus changed: " << oldName << "->" << newName;
415413

416414
auto* oldWorkspace = this->findWorkspaceByName(oldName);
417415
auto* newWorkspace = this->findWorkspaceByName(newName);
@@ -431,7 +429,7 @@ void I3Ipc::handleWorkspaceEvent(I3IpcEvent* event) {
431429
auto* oldWorkspace = this->findWorkspaceByName(name);
432430

433431
if (oldWorkspace != nullptr) {
434-
qCInfo(logi3Ipc) << "Deleting" << oldWorkspace->id() << name;
432+
qCInfo(logI3Ipc) << "Deleting" << oldWorkspace->id() << name;
435433

436434
if (this->mFocusedWorkspace == oldWorkspace) {
437435
this->setFocusedWorkspace(nullptr);
@@ -441,7 +439,7 @@ void I3Ipc::handleWorkspaceEvent(I3IpcEvent* event) {
441439

442440
delete oldWorkspace;
443441
} else {
444-
qCInfo(logi3Ipc) << "Workspace" << name << "has already been deleted";
442+
qCInfo(logI3Ipc) << "Workspace" << name << "has already been deleted";
445443
}
446444
} else if (change == "move" || change == "rename" || change == "urgent") {
447445
auto name = event->mData["current"]["name"].toString();
@@ -453,10 +451,10 @@ void I3Ipc::handleWorkspaceEvent(I3IpcEvent* event) {
453451

454452
workspace->updateFromObject(data);
455453
} else {
456-
qWarning() << "Workspace" << name << "doesn't exist";
454+
qCWarning(logI3Ipc) << "Workspace" << name << "doesn't exist";
457455
}
458456
} else if (change == "reload") {
459-
qInfo(logi3Ipc) << "Refreshing Workspaces...";
457+
qCInfo(logI3Ipc) << "Refreshing Workspaces...";
460458
this->refreshWorkspaces();
461459
}
462460
}
@@ -468,21 +466,21 @@ I3Monitor* I3Ipc::monitorFor(QuickshellScreenInfo* screen) {
468466
}
469467

470468
I3Workspace* I3Ipc::findWorkspaceByName(const QString& name) {
471-
auto mList = this->mWorkspaces.valueList();
472-
auto workspaceIter = std::find_if(mList.begin(), mList.end(), [name](const I3Workspace* m) {
469+
auto list = this->mWorkspaces.valueList();
470+
auto workspaceIter = std::find_if(list.begin(), list.end(), [name](const I3Workspace* m) {
473471
return m->name() == name;
474472
});
475473

476-
return workspaceIter == mList.end() ? nullptr : *workspaceIter;
474+
return workspaceIter == list.end() ? nullptr : *workspaceIter;
477475
}
478476

479477
I3Monitor* I3Ipc::findMonitorByName(const QString& name) {
480-
auto mList = this->mMonitors.valueList();
481-
auto monitorIter = std::find_if(mList.begin(), mList.end(), [name](const I3Monitor* m) {
478+
auto list = this->mMonitors.valueList();
479+
auto monitorIter = std::find_if(list.begin(), list.end(), [name](const I3Monitor* m) {
482480
return m->name() == name;
483481
});
484482

485-
return monitorIter == mList.end() ? nullptr : *monitorIter;
483+
return monitorIter == list.end() ? nullptr : *monitorIter;
486484
}
487485

488486
ObjectModel<I3Monitor>* I3Ipc::monitors() { return &this->mMonitors; }

0 commit comments

Comments
 (0)