-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add unit test for long user configured chords. #2736
Conversation
Travis tests have failedHey @regiontog, Node.js: 8npm test --silent
|
bec4a4c
to
82e8b7a
Compare
Hm, I don't quite recall the reasoning behind that piece of code. Your logic does seem to make sense though. |
@jpoon I guess the main assumption I'm making here is that a potential remap should alter the state in the same way a |
I don't recall the reasoning here either... |
Sorry @regiontog this has taken so long. My hesitation is that I don't recall the reasoning why we had that in the first place, but your PR makes sense so I'm inclined to merge it. What's your take @xconverge? |
@jpoon No problem at all, I only run into this problem when making weird keybinds for convenience functions. It seems odd to me that the extension works as well as it does if my reasoning is correct though, so I am cautious of this change as well. I don't know how many UT you guys have for user keybinds, but perhaps it would be good to have some more to increase confidence? |
I am ok to merge it and move on :) I can't really recall either way |
Yikes, that merge was a mess. Hope that resolves it. |
Travis tests have failedHey @regiontog, Node.js: 8npm run build
npm test --silent
|
What this PR does / why we need it:
Unit test replicating #2735
Which issue(s) this PR fixes
Special notes for your reviewer:
I don't understand the state here well enough to say if it's ok to return from
handleKeyEventHelper
like in f88f988. However I think the cause of the bug is when theNoPossibleMatch
action is returned fromActions.getRelevantAction
and there is a possible remap. In this case theNoPossibleMatch
action is ran, causingrecordedState.actionKeys
to be set to[]