Add Retro Shift (Auto Shift for Tap Hold via Retro Tapping)

[manna-harbour]

Description

Retro Shift (Auto Shift for Tap Hold via Retro Tapping) combines the Tap Hold logic of Retro Tapping with the configurable keycode range matching of Auto Shift while preserving the ability to hold and release without producing a tap.

Types of Changes

• Core
• Enhancement/optimization
• Documentation

Issues Fixed or Closed by This PR

• Resolves [Feature Request] Retro tapping should consider auto shift #9182.

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[IsaacElenbaas]

This looks great, and is much (much) cleaner than my implementation which I've been using hardcoded in process_record_user (plus doesn't need to be coded for each key). Making AS work with custom keys like this is awesome.

First off, this is not controllable by the Auto Shift toggle keys. Some just toggle AS if their base layers are normal enough to game on with it disabled. This could be fixed together with below, by calling AS itself.

My second concern is maintaining consistent behavior - there's currently an open pull for AutoShift keyrepeat and if we're making this an included feature I feel we should find a way to support that. That should be as simple as calling process_auto_shift (on key down and up) with an extra flag to let it know to not matrix timeout or send if another key cancels it. That also prevents the duplicate code. If RETRO_SHIFT is exceeded, don't call that on release and when the endlessly pending key is evaluated because of another down it won't be pressed due to the flag. The release would need to unregister the base key, too, as that pull relies on the key event for ending keyrepeat. I see no additional changes that would need to be made if custom shifts is added to that pull.

[drashna]

I've retargeted to the develop branch. You may want to rebase your PR.

[IsaacElenbaas]

I just tested, and the approach I outlined above works. I've not put it anywhere online yet, as the small changes to AS depend on whether custom shifts is merged in the other pull, but it's quite simple to throw together.

Not sure if we want to merge this and 'fix' it in the other pull when that's merged or just wait.

[manna-harbour]

I've retargeted to the develop branch. You may want to rebase your PR.

Thanks @drashna, it's now rebased onto develop.

[manna-harbour]

@IsaacElenbaas Thanks! This is independent of the Auto Shift implementation, and if that is changing soon I'd like to keep it separate for now. If this PR is accepted and the appropriate interfaces are added to Auto Shift later then this implementation could be modified to use them instead. That could include toggle, although if off it would then revert to normal tap hold behaviour which probably isn't useful for gaming anyway.

[IsaacElenbaas]

There's obviously no issue with merging this now, aside from cluttering that pull or having to make a new one. I just doubt many of the few using AS would notice and switch over in the time we would delay this but not be willing to work from a branch - my hope is that won't take too long to be merged lol. I'm not saying we should wait, just pointing it out.

normal tap hold behaviour which probably isn't useful for gaming anyway.

I disagree. Mod taps are just as great for gaming as they are everywhere else, even if only for rarely used actions. There are no reasons to have this enabled but not AS itself, so if this is merged before the adaptation I suggested is viable I think this should respect the AS enabled state if at all possible.

[noroadsleft]

This needs to be rebased upon develop and have the merge conflicts resolved.

[manna-harbour]

This needs to be rebased upon develop and have the merge conflicts resolved.

Thanks @noroadsleft, it's now rebased onto the force-pushed develop.

[manna-harbour]

Rebased after force-push to develop.

[tzarc]

Weak approval.

[manna-harbour]

Rebased after force-push to develop.

[manna-harbour]

If anyone would like to use this now, this branch is based on master instead and won't be rebased.

[IsaacElenbaas]

AS keyrepeat and early registration pull was just merged, so what I said here should definitely be done if only for firmware size (though behavior is also different now, with keyrepeat) before this is merged. I'm going to move my custom shifts pull and then am planning on opening a pull into this branch to make this call process_auto_shift. Might take a bit as finals are coming up and I want to make sure keyrepeat and custom shifts both work well with it.

If possible, could you rebase this again? Would make it easier to look through.

[IsaacElenbaas]

@manna-harbour said in Discord it would be fine for me to open the above pull due to many conflicts with this and custom shifts, which now has basic implementation. Retro Shift shouldn't need much more work, but I won't be able to do anything to it for a while.

action_tapping.c sucks - I tried to implement this using it, spert seven hours on it, then realized I didn't even need it and got it redone in two.

EDIT: then stared at action_tapping.c for five hours and then worked with it because the first was horribly janky lol

[drashna]

Looks like this needs a rebase.

[IsaacElenbaas]

The retro shift part of that is ready. IMO this should be closed now - it adds twice the lines (I'm thinking of firmware size), doesn't work with TAPPING_TERM higher than AUTO_SHIFT_TIMEOUT, and doesn't support keyrepeat.