-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(hyprland): detect dispatch protocol via configProvider #5231
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,11 +10,13 @@ | |
| #include <unistd.h> | ||
|
|
||
| #include <array> | ||
| #include <cctype> | ||
| #include <cerrno> | ||
| #include <cstring> | ||
| #include <filesystem> | ||
| #include <optional> | ||
| #include <string> | ||
| #include <string_view> | ||
|
|
||
| #include "util/scoped_fd.hpp" | ||
|
|
||
|
|
@@ -292,48 +294,58 @@ Json::Value IPC::getSocket1JsonReply(const std::string& rq) { | |
| return parser_.parse(reply); | ||
| } | ||
|
|
||
| bool IPC::isLuaProtocol() { | ||
| if (s_luaProtocolDetected_.has_value()) { | ||
| return *s_luaProtocolDetected_; | ||
| std::optional<bool> IPC::parseConfigProvider(const std::string& systemInfo) { | ||
| // Hyprland reports which config manager it actually loaded in "systeminfo", | ||
| // as a "configProvider: lua" / "configProvider: legacy" line. That is the | ||
| // authoritative signal: the version alone is not enough, because Hyprland | ||
| // only uses the Lua manager when the config file name ends in ".lua", so a | ||
| // >= 0.54 instance started with a traditional hyprland.conf still speaks the | ||
| // legacy dispatch protocol. | ||
| static constexpr std::string_view key = "configProvider:"; | ||
|
|
||
| const size_t keyPos = systemInfo.find(key); | ||
| if (keyPos == std::string::npos) { | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| // Detect the Lua-based dispatch protocol (Hyprland >= 0.54) via the read-only | ||
| // "version" query. This MUST have no side effects: an earlier probe issued a real | ||
| // "dispatch workspace __waybar_probe__", which on Hyprland < 0.54 actually switched | ||
| // the user to a junk workspace named __waybar_probe__ on the first click/scroll. | ||
| bool luaProto = false; | ||
| try { | ||
| util::JsonParser parser; | ||
| const Json::Value ver = parser.parse(getSocket1Reply("j/version")); | ||
|
|
||
| // Prefer the numeric "version" field ("0.54.0"); fall back to the "tag" field | ||
| // ("v0.54.0" or "v0.54.0-16-gdeadbee"), which is present on all releases. | ||
| std::string versionStr = ver["version"].asString(); | ||
| if (versionStr.empty()) { | ||
| versionStr = ver["tag"].asString(); | ||
| } | ||
| size_t valuePos = systemInfo.find_first_not_of(" \t", keyPos + key.size()); | ||
| if (valuePos == std::string::npos) { | ||
| return false; | ||
| } | ||
|
|
||
| const size_t firstDigit = versionStr.find_first_of("0123456789"); | ||
| if (firstDigit != std::string::npos) { | ||
| // std::stoi parses the leading integer and stops at the first non-digit, so it | ||
| // tolerates the trailing ".patch-commits-ghash" suffix on the tag. | ||
| const int major = std::stoi(versionStr.substr(firstDigit)); | ||
| int minor = 0; | ||
| const size_t dot = versionStr.find('.', firstDigit); | ||
| if (dot != std::string::npos && dot + 1 < versionStr.size()) { | ||
| minor = std::stoi(versionStr.substr(dot + 1)); | ||
| } | ||
| luaProto = major > 0 || (major == 0 && minor >= 54); | ||
| } else { | ||
| spdlog::warn("Hyprland IPC: could not parse version '{}', assuming legacy protocol", | ||
| versionStr); | ||
| } | ||
| const size_t end = systemInfo.find_first_of("\r\n", valuePos); | ||
| std::string provider = systemInfo.substr(valuePos, end - valuePos); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a check for end == std::string::npos. This code will usually work (unless someone tries to build it on some weird platform), but I'm not sure if it's considered as a good pattern. std::string::npos is defined as
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made it an explicit branch in 92742e4: |
||
| while (!provider.empty() && std::isspace(static_cast<unsigned char>(provider.back()))) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. include/util/string.hpp contains an rtrim function, would that be sufficient?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better than sufficient — switched to |
||
| provider.pop_back(); | ||
| } | ||
|
|
||
| spdlog::debug("Hyprland IPC: systeminfo reports configProvider '{}'", provider); | ||
| return provider == "lua"; | ||
| } | ||
|
|
||
| std::optional<bool> IPC::luaProtocolFromSystemInfo() { | ||
| try { | ||
| return parseConfigProvider(getSocket1Reply("systeminfo")); | ||
| } catch (const std::exception& e) { | ||
| spdlog::warn("Hyprland IPC: version detection failed ({}), assuming legacy protocol", e.what()); | ||
| spdlog::warn("Hyprland IPC: could not read configProvider from systeminfo ({})", e.what()); | ||
| return std::nullopt; | ||
| } | ||
| } | ||
|
|
||
| bool IPC::isLuaProtocol() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure that this function needs to be split into 3? In this case I feel like it increases complexity and makes code harder to read. I suppose it makes it slightly easier to write UTs, but imo it's not a significant difference.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Collapsed to two in 92742e4: |
||
| if (s_luaProtocolDetected_.has_value()) { | ||
| return *s_luaProtocolDetected_; | ||
| } | ||
|
|
||
| // configProvider landed together with the Lua config manager in 0.55, so its | ||
| // absence means the instance predates Lua support entirely and necessarily | ||
| // speaks the legacy protocol. That makes the field sufficient on its own, and | ||
| // it is read-only, so detection has none of the side effects of an actual | ||
| // dispatch probe. | ||
| const bool luaProto = luaProtocolFromSystemInfo().value_or(false); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth it to have those functions return an optional bool considering that in the end nullopt follows the same path as false?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right, and the history explains how it got there: the tri-state existed to route "field absent" to the version fallback. Once Mrpaoo's comment removed that fallback, nullopt and false converged on the same path and the optional answered nothing the caller could act on. Dropped in 92742e4 — plain bool, absent field documented as "pre-Lua instance, hence legacy". |
||
|
|
||
| if (luaProto) { | ||
| spdlog::info("Hyprland IPC: detected Lua-based dispatch protocol (Hyprland >= 0.54)"); | ||
| spdlog::info("Hyprland IPC: detected Lua-based dispatch protocol"); | ||
| } else { | ||
| spdlog::info("Hyprland IPC: detected legacy dispatch protocol"); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
The name
parseConfigProvidersuggests that the function returns a parsed config provider, whereas it actually answers the question "is the config provider Lua?". Since the return type isstd::optional<bool>it would be better to use a name similar toisLuaProtocol, where the boolean states are clearly understandable.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.
Agreed — renamed to
isLuaConfigProviderin 92742e4, so the bool it returns is the question the name asks.