Skip to content

Conversation

@rfresh2
Copy link
Contributor

@rfresh2 rfresh2 commented Sep 28, 2025

Describe your PR, what does it fix/add?

repeat bug:

binde = SUPER, a, sendshortcut, , b,
  1. hold super + a until repeat
  2. release a
  3. b continues being entered

long press bug:

bindo = SUPER, c, sendshortcut, d,
  1. press super + c
  2. release c before long press trigger
  3. d is entered

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

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

ready

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs tests

@rfresh2
Copy link
Contributor Author

rfresh2 commented Sep 28, 2025

i will need some hand-holding through how to create the tests

or i'm fine handing this off, up to you.

@vaxerski
Copy link
Member

@rfresh2
Copy link
Contributor Author

rfresh2 commented Sep 29, 2025

i added some tests

but i cherry-picked the tests back down to main and they also pass

i double checked the bug is still indeed reproducible on main through manual tests

so something is off with how keybinds are applied in the test plugin vs prod

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's ok, timings in the runner VM are very inconsistent and unreliable. If they pass, it's ok.

@rfresh2
Copy link
Contributor Author

rfresh2 commented Sep 29, 2025

ok i found the issue

wasn't talking about CI runners or tests being flakey

this codepath is being hit by exec on key release but not sendshortcut because it is a "SPECIALDISPATCHER"

@vaxerski
Copy link
Member

vaxerski commented Oct 1, 2025

well tests have to pass, you can either remove those that can't be done or try to fix them

@rfresh2
Copy link
Contributor Author

rfresh2 commented Oct 1, 2025

was able to instrument sendshortcut tests with kitty

there's still some weird behavior but the tests are at least catching the original bugs

@vaxerski
Copy link
Member

vaxerski commented Oct 3, 2025

great! thanks

vaxerski
vaxerski previously approved these changes Oct 3, 2025
@vaxerski
Copy link
Member

vaxerski commented Oct 3, 2025

oops c-f failed

@rfresh2
Copy link
Contributor Author

rfresh2 commented Oct 4, 2025

fixed

@vaxerski
Copy link
Member

vaxerski commented Oct 4, 2025

now test failed lol

@rfresh2
Copy link
Contributor Author

rfresh2 commented Oct 4, 2025

added a polling wait thingy for the prompt now

@ItsOhen
Copy link
Contributor

ItsOhen commented Oct 6, 2025

Are we there yet? :))
Wanted to add some tests to submaps, but figured i would wait until this is in so i don't have to update it later.

@vaxerski vaxerski merged commit 73f0643 into hyprwm:main Oct 6, 2025
13 checks passed
Mozzarella32 pushed a commit to Mozzarella32/Hyprland that referenced this pull request Oct 8, 2025
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.

3 participants