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

Fix movtodesk dispatchers #74

Merged
merged 2 commits into from
Dec 19, 2024
Merged

Fix movtodesk dispatchers #74

merged 2 commits into from
Dec 19, 2024

Conversation

WhySoBad
Copy link
Collaborator

Hi,

Since the switch from std::regex to re2 as a regex library for hyprland (hyprwm/Hyprland@e06b520) the movetodesk dispatcher(s) are kind of broken.

For example with the config:

plugin {
    virtual-desktops {
        cycleworkspaces = 0
        notifyinit = 0
        names = 1:1, 2:2, 3:3, 4:4, 5:5
    }
}

I previously was able to switch vdesks using dispatch movetodesk <id> where <id> is either 1, 2, 3, 4 or 5.

When I dispatch movetodesk 2 without further arguments we have arg = '' in the VirtualDeskManager::moveToDesk method.

PHLWINDOW window = g_pCompositor->getWindowByRegex(arg);
if (!window) {
printLog(std::format("Window {} does not exist???", arg), eLogLevel::ERR);
return vdeskId;
}

We then call getWindowByRegex(arg) which now returns no window (probably due to us passing an empty string and re2 not liking this in comparison to std::regex which most likely just returned the current window). This causes the assertion and therefore every move to fail.

I've now wrapped the whole assertion into a check whether arg is non-empty. Since we only use the window from getWindowByRegex to determine the target monitor we set monitor to the current monitor by default. Should arg be non-empty we try to get a window matching the arg and set monitor to the monitor of the matched window. If no window matches we currently just return and don't move anything. I think it would be better when we log some error and then move to the default monitor (which is the current monitor) as it's probably more like the functionality which was tried to be achieved by the user.

I'm not entirely sure whether this could break some control flows as I don't have this deep of an understanding of the functionalities of the plugin which I only rarely use

@WhySoBad WhySoBad self-assigned this Dec 18, 2024
@levnikmyskin
Copy link
Owner

LGTM, don't think this should break anything. The new Hyprland broke window rules anyway, so I guess this is expected

@levnikmyskin levnikmyskin merged commit 4e4e9b0 into dev Dec 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants