feat(behaviors): Hold-tap - Hold while undecided

[nickconway]

Addresses #1393

[Sixzero]

Wow, we even can merge it soon? You guys are crazy fast! Loving it!

[Sixzero]

I am trying to understand why the modkey getting stuck with this option, but I don't see a problem in your code.

[Sixzero]

If someone would review the pull, I would suggest it to get merged. :) The solution seems to be working perfectly! Also any code that was added, only runs if this flavor is turned on, so it won't have effect on other things.

[Sixzero]

Could someone review and approve this? Would be good if it is out in official! It has been working like a charm since the last update! The readme of zmk changed on master AFAIK, so maybe that will need a little evident correction.

[Sixzero]

Could we get it reviewed. I have been using it since aug 1, and everything is working perfectly.
It really is and important feature, not just for me but for anyone trying to use SHIFT + mouse.

[petejohanson]

@okke-formsma Thoughts on this?

[petejohanson]

Style issue... we use snake_case variable names.

[petejohanson]

Suggested change

	(hold_tap->status == STATUS_HOLD_TIMER || hold_tap->status == STATUS_HOLD_INTERRUPT) &&
	decision_moment == HT_KEY_UP;
	!key_tap && decision_moment == HT_KEY_UP;

[okke-formsma]

This seems to be the only place where 'first_press' is used and I do not understand why it is needed at all. What does this code solve? Why should we not capture the events that would be captured on line 697?

[okke-formsma]

I do not feel this MR is on the right track implementation-wise. It adds two new state variables (first_pressed and hold_released) while completely ignoring the existing status variable.

I would suggest to implement this in the following way, more in line with the original implementation of hold-tap. It uses the existing status variable and existing machinery to centralise decisionmaking.

• add a new status enum value STATUS_UNDECIDED_INITIAL_HOLD
• add a new function status_is_undecided which returns true if a status is STATUS_UNDECIDED or STATUS_UNDECIDED_INITIAL_HOLD. Replace all tests for STATUS_UNDECIDED with this new function.
• add a new decision_moment enum value HT_KEY_DOWN. Call decide_hold_tap on initial press with this new value.
• add a new function decide_initial_hold which is called at the top of decide_hold_tap which sets STATUS_UNDECIDED_INITIAL_HOLD when appropriate and presses the hold key binding.
• modify press_binding. If 'hold' is decided, do not press the hold again when initial_hold is enabled. If 'tap' is decided, make sure to release the initial hold.

[Sixzero]

@okke-formsma your idea, of trying to solve the functionality without that 2 extra states, seems to be an important request. I also think the functionality should be more deeply integrated, while I think Nicon thought things would be more modular if he used these two states.

I can't reach @nickconway on discord anymore, I have tried to do so in the last 3 weeks. I don't know what is happening to him, so probably the correction of the pull request is on us.

[sporkus]

Just want to add that this PR works perfectly for me. I am using this for shift, gui(Mac) and ctrl(Windows). Without this option, homerow mods aren't very usable with mouse clicks.

[Sixzero]

I am using it also, and shift + mouseclicks are insanely good! Loving it so much! I hope some day we will merge it!

[theol0403]

Close this in favor of #1811