Skip to content
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

Is there a version of tap-next that affects only one subsequent press? #166

Closed
ghost opened this issue Jan 2, 2021 · 12 comments · Fixed by #167
Closed

Is there a version of tap-next that affects only one subsequent press? #166

ghost opened this issue Jan 2, 2021 · 12 comments · Fixed by #167
Labels
bug Something isn't working

Comments

@ghost
Copy link

ghost commented Jan 2, 2021

I would like to have a key that causes the next-pressed key to be capitalised but not any subsequent key, even if I (usually accidentally) press that subsequent key before releasing the previous key.

Example:

I have z bound to @nsh

I have nsh declared as follows:

nsh (around-next sft)

Then I try to type "Am I blue?"

If I do Pz Rz Pa Ra Pm Rm then everything is fine.

But often I accidentally do Pz Rz Pa Pm Ra Rm, and then I get "AM I blue?"

I could learn to type more carefully, but it seems to me that this is something kmonad could take care of for me if there was a command I could use like around-next but which acted only on the next key and not any subsequent key.

Am I missing something?

@slotThe
Copy link
Member

slotThe commented Jan 2, 2021

As far as I can tell, this is a bug. The documentation clearly says

The around-next function creates a button that primes KMonad to
perform the next button-press inside some context.

"Next button", not "next few buttons until the button that was pressed
after me is released" ;)

There is an obvious "fix" here, but I'm not sure if this breaks
anything. It would certainly be a regression (though maybe we can wave
that away with "the previous behaviour was a bug"), as something like

Tz Pa PCtrl Ra RCtrl ==> S-C-a

would cease to work.

The "fix" would be as follows:

diff --git a/src/KMonad/Button.hs b/src/KMonad/Button.hs
index eb70146..040e8a0 100644
--- a/src/KMonad/Button.hs
+++ b/src/KMonad/Button.hs
@@ -178,9 +178,9 @@ around outer inner = Button
 aroundNext ::
      Button -- ^ The outer 'Button'
   -> Button -- ^ The resulting 'Button'
-aroundNext b = onPress $ await isPress $ \e -> do
+aroundNext b = onPress $ await isPress $ \_ -> do
   runAction $ b^.pressAction
-  await (isReleaseOf $ e^.keycode) $ \_ -> do
+  await ((||) <$> isPress <*> isRelease) $ \_ -> do
     runAction $ b^.releaseAction
     pure NoCatch
   pure NoCatch

@slotThe slotThe added the bug Something isn't working label Jan 2, 2021
@ghost
Copy link
Author

ghost commented Jan 2, 2021

Thank you @slotThe!

I don't suppose backwards compatibility is important yet - there are not many of us using kmonad yet.

For me personally, it would be just as good to have a new function that did what I want ... but then someone would have to deal with the extra complexity in the documentation. So, for that reason, I think your solution is probably best.

@MaxGyver83
Copy link
Contributor

This sounds like a bug to me, too.

But: I don't use the physical modifier keys anymore. Only letter keys with modifiers via tap-hold-next-release. If you (@slotThe ) change the code as proposed, will I still be able to press C-S-c on the home row (with Ctrl and Shift using tap-hold-next-release)? I suppose tap-hold-next-release is not affected. I just want to be sure.

@slotThe
Copy link
Member

slotThe commented Jan 2, 2021 via email

@slotThe
Copy link
Member

slotThe commented Jan 2, 2021

@eighal can you confirm that the fix in #167 works for you as well?

@ghost
Copy link
Author

ghost commented Jan 2, 2021

You say that, but this repo has over 400 stars by now. We even have packages for obscure distributions!

I'm pleased to have been wrong about that!

@ghost
Copy link
Author

ghost commented Jan 2, 2021

@eighal can you confirm that the fix in #167 works for you as well?

I've pretty much never used github except for issues pages. Could you please tell me how to download the source tree with that fix? (Once I've got the source I know how to compile it.)

Thank you!

@slotThe
Copy link
Member

slotThe commented Jan 2, 2021

@eighal can you confirm that the fix in #167 works for you as well?

I've pretty much never used github except for issues pages. Could you please tell me how to download the source tree with that fix? (Once I've got the source I know how to compile it.)

I don't think github has any kind of special functionality for that. What you have to do is clone my "fork" of the repo, and checkout the appropriate branch, i.e.

git clone https://github.com/slotthe/kmonad
cd kmonad
git checkout origin/around-next

If you're really old school, you can also get the patch by appending .patch (or the raw diff by appending .diff) to the pull request URL

@MuhammedZakir
Copy link
Contributor

MuhammedZakir commented Jan 2, 2021

You can also do:

git clone https://github.com/david-janssen/kmonad
cd kmonad
git fetch origin pull/167/head:around-next-167
git checkout around-next-167

@ghost
Copy link
Author

ghost commented Jan 2, 2021

Thank you @slotThe and @MuhammedZakir.

@slotThe, works for me. Awesome! Thank you.

I hope @david-janssen is happy to merge this.

@david-janssen
Copy link
Collaborator

Hey everyone,

Sorry again for being gone (and still not reliably present), I think this was a good fix, but I do actually want to stay backward compatible. I always get annoyed at software when a non-major update breaks my config. However, I fully support the idea of adding a second button, perhaps around-next-single?

Actually, that sounds like just the small little fix that I could get started with. Will have a look at this now.

@MagicDuck
Copy link

MagicDuck commented Jun 27, 2021

@david-janssen I am running into the same problem with around-next still. Looking a the code base, KAroundNextSingle exists, but around-next-single was not defined in Parser.hs and Tokenizer.hs. Was it removed for some reason?
Note, in case somebody else runs into the issue, adding the following line to both Parser.hs and Tokenizer.hs under ("around-next" , KAroundNext <$> buttonP) will allow you to use around-next-single:

  , ("around-next-single"    , KAroundNextSingle  <$> buttonP)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants