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

Fix being unable to type uppercase characters on Windows #285

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kirillsemyonkin
Copy link

Fixes the problem with Windows being unable to type out uppercase characters when using Key::Unicode keys (e.g. Key::Unicode('T')).

Upon using the key function that types out Unicode keys for uppercase characters, the call returns the error below:

error when mapping keycode to keysym: (Could not translate the character to the corresponding virtual-key code and shift state for the current keyboard)

Apparently the call for VkKeyScanW in enigo's code does not handle the uppercase characters: the code is translated into two bytes (as huge comment above the call mentions) as a single u32 (upper u16 of which isn't used?), which later was not handled and passed over to MapVirtualKeyW, which fails because it cannot translate two-byte value (e.g. 340 for 'T') into a scancode. This PR handles the two bytes by emitting extra necessary scancodes (e.g. SCANCODE_LSHIFT) depending on the flags in the high-order byte.

P.S. Why does Windows implementation mention keysym in its errors?

@kirillsemyonkin
Copy link
Author

Mind that for this PR I have not checked for Issues or other PRs or branches - I just did what happened to fix my problem, and would like to see it up on Cargo crates.io as soon as possible (it is a major feature that does not work on Windows without this fix), so that I can use it in a project I would like to publish soon-ish.

@kirillsemyonkin
Copy link
Author

kirillsemyonkin commented May 3, 2024

Something to probably consider for the future: these non-...ExW calls probably use the current (e.g. en-us) keyboard layout, which probably means it will be unable to type out keys from other languages, unless user explicitly switches layout (or an Alt+Shift combination is generated - which is configurable on many OSes and, thus, unusable in crossplatform way). The ...ExW allows to input a particular keyboard layout (which has to be preloaded with a LoadKeyboardLayoutW), which then I, honestly, have no idea how to figure out which layout the unicode char belongs to (maybe iterate over user's layouts and select non-failing mapping only from those?).

@pentamassiv
Copy link
Collaborator

Thank you very much for your PR. I hope to have time to test it in the beginning of next week.

Regarding the mapping of keys, I created a PR to check the current layout of the user and use that #280

@pentamassiv
Copy link
Collaborator

I didn't have time to test it with the borrowed machine but will have access to it at the end of may again. Hopefully I can test it then.

@kirillsemyonkin
Copy link
Author

I can give you a remote Windows VM if you want

@pentamassiv
Copy link
Collaborator

Thank you for your kind offer, but unfortunately I have been too busy to work on enigo anways.

I finally tested your code and it works just as you described. After thinking about it a bit more, I am a little hesitant to merge it though. You are already able to type uppercase letters on Windows by using the text method or explicitly simulating a Shift press and release like so:

use enigo::{
    Direction::{Click, Press, Release},
    Enigo, Key, Keyboard, Settings,
};
use std::thread;
use std::time::Duration;

fn main() {
    env_logger::init();
    thread::sleep(Duration::from_secs(2));
    let mut enigo = Enigo::new(&Settings::default()).unwrap();

    // Option A
    enigo.text("T").unwrap();

    // Option B
    enigo.key(Key::Shift, Press).unwrap();
    enigo.key(Key::Unicode('t'), Click).unwrap();
    enigo.key(Key::Shift, Release).unwrap();
}

Merging your suggested changes would have the upside that you would not have to explicitly simulate the Shift key, but it would have the downside, that it would release the Shift key regardless of its state before simulating an uppercase letter. Consider the following code for example:

use enigo::{
    Direction::{Press, Release,Click},
    Enigo, Key, Keyboard, Settings,
};
use std::thread;
use std::time::Duration;

fn main() {
    env_logger::init();
    thread::sleep(Duration::from_secs(2));
    let mut enigo = Enigo::new(&Settings::default()).unwrap();

    enigo.key(Key::Shift, Press).unwrap();
    enigo.key(Key::Unicode('T'), Press).unwrap();
    thread::sleep(Duration::from_secs(1));
    enigo.key(Key::Unicode('T'), Release).unwrap();
    
    enigo.key(Key::Unicode('t'), Click).unwrap();
    enigo.key(Key::Shift, Press).unwrap();
}

What should be the result of this? Should it be Tt, TT, should it throw an error? I am not sure what is best here. From a user perspective I think it would be best if it would result in TT but to do that, we have to check the state of the modifier keys before and restore it afterwards.

Do you have any thoughts on this?

@kirillsemyonkin
Copy link
Author

I thought you were asking, if I did shift followed by lowercase unicode t, would it preserve shift or would it reset to released, I think latter is the behavior I would expect as a user: assuming using Unicode resets modifiers to whatever it needs (i.e. Unicode being a shortcut to key sequences like shift,letter,unshift, last one always happening).

Now I look at example and I see; I would definitely prefer lowercase t Unicode to type a lowercase t no matter what, because its for typing specific characters, not just pressing keyboard keys labelled as them.

If you can document it, maybe supply with examples (would they as doctests be untestable?), I think any behavior goes.

You can also add multiple Unicode variants, or a struct variant with options.

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.

2 participants