Skip to content

Conversation

@kolayne
Copy link
Contributor

@kolayne kolayne commented Sep 15, 2025

Describe your PR, what does it fix/add?

The dispatcher gesture handler used to only handle
the first argument to the dispatcher, while some dispatchers
(e.g., sendshortcut) want multiple arguments.

This fixes ConfigManager to handle all the arguments
provided to the dispatcher gesture handler.

Fixes #11684.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

An alternative implementation could be to join the already parsed arguments with the , separator. But using the original string feels more portable, e.g., in case future dispatchers may depend on whitespace, etc.

Is it ready for merging, or does it need work?

Hopefully, ready for merge!

@kolayne
Copy link
Contributor Author

kolayne commented Sep 16, 2025

Huh. The code style pipeline fails for a line that I never edited.

@kolayne kolayne force-pushed the gesture-dispatcher-11684 branch 2 times, most recently from 61b1002 to d7bc4a0 Compare September 17, 2025 00:57
The `dispatcher` gesture handler used to only handle
the first argument to the dispatcher, while some dispatchers
(e.g., `sendshortcut`) want multiple arguments.

This fixes `ConfigManager` to handle all the arguments
provided to the dispatcher gesture handler.

Fixes hyprwm#11684.
Reduce code duplication in the gestures test.
@kolayne kolayne force-pushed the gesture-dispatcher-11684 branch from d7bc4a0 to 343a4ad Compare September 17, 2025 12:47
@vaxerski vaxerski merged commit 41dad38 into hyprwm:main Sep 20, 2025
13 checks passed
@kolayne kolayne deleted the gesture-dispatcher-11684 branch September 21, 2025 01:00
@kolayne
Copy link
Contributor Author

kolayne commented Sep 21, 2025

@vaxerski, ahh, unfortunately, this MR breaks something. Consider the line:

gesture = 4, left, dispatcher, movefocus, l

According to the code, we find the last comma and take the first character after it, which is a space. The moveFocusTo function ends up receiving the string " l", and the first character of it is not recognized as a direction, so the binding does not work.

I want to submit a fix, but I am not sure which function's fault is it: are dispatchers supposed to handle whitespaces, so we need to update moveFocusTo? Or is it config manager's responsibility to strip whitespaces?

@kolayne
Copy link
Contributor Author

kolayne commented Sep 21, 2025

I can see that CVarList and CConstVarList have join methods, so we can keep the parsing logic there and just join the rest of the arguments parsed with the gesture keyword. I will do that.

However, should dispatcher functions accept std::string at all? Maybe it makes more sense to instead accept a range of parsed strings, in the form of a CVarList/CConstVarList or a pair of iterators over std::string_view, etc?

ItsOhen pushed a commit to ItsOhen/Hyprland that referenced this pull request Sep 21, 2025
* config: Fix multi-argument gesture dispatchers parsing

The `dispatcher` gesture handler used to only handle
the first argument to the dispatcher, while some dispatchers
(e.g., `sendshortcut`) want multiple arguments.

This fixes `ConfigManager` to handle all the arguments
provided to the dispatcher gesture handler.

Fixes hyprwm#11684.

* test/gestures: Add a test for a gesture with a multi-argument dispatcher

* test/gestures: Factor out `waitForWindowCount`

Reduce code duplication in the gestures test.
littleblack111 pushed a commit to littleblack111/Hyprland that referenced this pull request Sep 23, 2025
* config: Fix multi-argument gesture dispatchers parsing

The `dispatcher` gesture handler used to only handle
the first argument to the dispatcher, while some dispatchers
(e.g., `sendshortcut`) want multiple arguments.

This fixes `ConfigManager` to handle all the arguments
provided to the dispatcher gesture handler.

Fixes hyprwm#11684.

* test/gestures: Add a test for a gesture with a multi-argument dispatcher

* test/gestures: Factor out `waitForWindowCount`

Reduce code duplication in the gestures test.
Mozzarella32 pushed a commit to Mozzarella32/Hyprland that referenced this pull request Oct 8, 2025
* config: Fix multi-argument gesture dispatchers parsing

The `dispatcher` gesture handler used to only handle
the first argument to the dispatcher, while some dispatchers
(e.g., `sendshortcut`) want multiple arguments.

This fixes `ConfigManager` to handle all the arguments
provided to the dispatcher gesture handler.

Fixes hyprwm#11684.

* test/gestures: Add a test for a gesture with a multi-argument dispatcher

* test/gestures: Factor out `waitForWindowCount`

Reduce code duplication in the gestures test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants