Skip to content

Conversation

@bfritscher
Copy link
Contributor

After looking into the code as I understand it the show only shortcuts was meant to only display key pressed which are listed in the keymaps yaml files and it is per process. The naming is misleading.
As evidenced by issue #94

But in the process of adding the merge function and reusing the Message(IEnumerable keys, KeyShortcut shortcut) constructor. All the merged text and x times got a isShortcut=true.
Therefore in the current state Only Shortcut shows everything except modifier keys without keympas file and individual characters. By adding a third argument which is only passed from the ShortcutAccumulator the original function is restored.

But this does not solve the problem to have a feature to only show "shortcut keys" meaning any combination of alt,ctrl,shift, windows keys. Those are also called modifiers keys. So I added an option to only display modifiers keys throughout any process.

A long time since I have done C#, any suggestions/improvements are welcome

@wickedviking
Copy link

This is a winner. I'm using it now due to #94

@hnrkndrssn
Copy link
Contributor

hnrkndrssn commented Jul 3, 2017

Apologies for the delay in reviewing this PR, I will try and get to it in the next day or so!

Thanks,
Henrik

@hnrkndrssn hnrkndrssn self-requested a review July 3, 2017 11:30
@hnrkndrssn hnrkndrssn self-assigned this Jul 3, 2017
@bfritscher
Copy link
Contributor Author

Hi, no problem in the meantime on my master i made a inital test of implementing mouse cursor highlight, but it needs some tweaking. Didn't see that I made the pull request on master instead of commit :-(

@hnrkndrssn
Copy link
Contributor

hnrkndrssn commented Jul 3, 2017

@bfritscher you can create a new branch of your master branch and once this PR has been merged your other PR should merge without issue as well. (Just mention it in your other PR as a note for the reviewer so they are aware that it depends on this PR) Oh, I see that you already pushed the changes and they made their way into this PR.

To fix it up, you can follow these instructions to move you're mouse cursor changes to its own branch.

@hnrkndrssn
Copy link
Contributor

@bfritscher , it looks like this PR now actually includes fixes for 3 separate issues:

It'd be good to split these out into 3 separate PRs to make code reviewing somewhat simpler.

@bfritscher
Copy link
Contributor Author

@alfhenrik yes, I had to quickly do some stuff for my use case the last two weeks. Now that the workshop is finished, I should have some time to cleanup and make the proper pull request on separate branches. Thanks for the pointers to the issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants