Windows keyboard layout handling: get the current layout from the parent terminal process

[ForNeVeR]

PR Summary

This is my another attempt on #1393, with a different approach this time.

Fixes #1393 in my environment somewhat: we'll use the proper current keyboard layout from the terminal process, which means the Ctrl+C will work in English layout and won't work in Cyrillic. But that's a step forward, since before it was not working at all if you started the process in non-English layout.

The terminal process lookup is a heuristic (since there's no official way to determine which process provides a terminal for you) and won't work in 100% of cases (because in some convoluted cases, the process/terminal/console ownership structure may be different). But it should be a good and stable improvement for most of the PowerShell use cases.

After we've got the parent terminal process (which I currently define as a parent process in the process tree that has its main window visible), we ask the OS of its current keyboard layout, and then pass it to ToUnicodeEx (since ToUnicode is hard-coded to always cache the wrong keyboard layout — the one we've called it the first time with).

Open issues: this only works well in Windows Terminal, IntelliJ and VSCode, but doesn't work in conhost. Perhaps let's leave it as it is for now?

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Make sure you've added one or more new tests
  • Impossible, unfortunately. This kind of behavior is very hard to test.
• Make sure you've tested these changes in terminals that PowerShell is commonly used in (i.e. conhost.exe, Windows Terminal, Visual Studio Code Integrated Terminal, etc.)
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed at PowerShell-Docs
    • Doc Issue filed:
  • I'm not sure, folks, please help me to figure it out

Microsoft Reviewers: Open in CodeFlow

[ForNeVeR]

Friendly ping. Folks, could you please help with the review here?

[daxian-dbw]

@jazzdelightsme, if you have time, can you please help review this PR? Thanks!

[lzybkr]

It's been a very long time since I last looked at this problem, but the code and approach seem sound to me.

It'd be nice to see it fixed for conhost since folks still use it, but knowing the problem is fixed in Windows Terminal might get some folks to switch.

[ForNeVeR]

I am open to ideas on how to fix it for conhost.

I've tried googling that but haven't found any working solutions on how to properly extract the active keyboard layout from conhost.

All we need is a way to get the active layout for a conhost process. If we can get that, then everything else is just a matter of determining whether we are attached to a conhost or not (and I believe this is possible to determine, even if a little tricky and will require another heuristic).

Just for the record, I use this version of PSReadline daily for the last several releases in VSCode, IntelliJ and Windows Terminal, and the functionality works reliably for me: it never crashed and never misdetected the terminal host.

[jazzdelightsme]

I know some experts in this area; I will look into it...

[daxian-dbw]

Have you tried using GetForegroundWindow?

I changed the GetConsoleKeyboardLayout method to the following and it seems working fine for me to retrieve the current active keyboard layout, even when pwsh is opened in Conhost from cmd.exe.

public static IntPtr? GetConsoleKeyboardLayout()
{
    IntPtr handle = GetForegroundWindow();
    if (handle != IntPtr.Zero)
    {
        uint tid = GetWindowThreadProcessId(handle, out _);
        if (tid > 0)
        {
            IntPtr layout = GetKeyboardLayout(tid);
            return layout;
        }
    }

    return null;
}

[ForNeVeR]

Thanks for your useful feedback! Will take a look tonight.

[ForNeVeR]

@daxian-dbw, thanks for your fantastic feedback. I've applied all the suggestions (the most significant change being the migration to GetForegroundWindow), and everything I was able to check still works well!

Notably, I no longer can reproduce the issue with conpty on the latest stable build of Windows 11 (22H2): that one seems to always work (even in non-English keyboard layout), so I guess that's fine by me. (Other terminal apps, including Windows Terminal, still require the workaround from this PR.)

[ForNeVeR]

Thanks for your useful feedback again! I believe I've addressed the suggestions. Please take another look!

[daxian-dbw]

@ForNeVeR I spent 2 days playing with code and decided that we should move back to your MainWindowHandle approach for two reasons:

1. GetForegroundWindow doesn't work for conhost either. The returned window handle is the same as the GetConsoleWindow call -- it's the window created for the console application (owner is cmd or pwsh for example), not the terminal window (owner is conhost I guess) that can really reflect keyboard layout changes. I cannot see the layout change by using the window handle returned from GetForegroundWindow when pwsh is running under conhost.

2. For the scenarios where GetForegroundWindow returns the correct window handle, walking up the process tree is also needed in order to verify if it's the correct one. It turns out the MainWindowHandle approach works for those scenarios too, and is more reliable because we do have the risk of race condition when using GetForegroundWindow.

So, I changed the code back and did some polishing. Now, about the GetTerminalOwnerThreadId method:

• It should continue to work well in Windows Terminal (with profile), IntelliJ and VSCode;
• It doesn't work when PowerShell runs in conhost, or when it gets started from Start Menu (or win+r "Run") with Windows Terminal as the default terminal application (without profile).
• For the scenarios where it doesn't work, I make sure it returns 0, so that the behavior is unchanged.

It's not perfect, but I'm afraid there is no perfect solution that can work in all scenarios.
I'm willing to take this improvement to unblock the Russian keyboard layout users. Can you please take a look at my changes and verify again that it still works for you in Windows Terminal (with profile), IntelliJ and VSCode? Please let me know if you have any concerns about my changes. Thanks!

[ForNeVeR]

@daxian-dbw, thank you so much for your help.

I can confirm the following observations:

• the resulting solution works well in
  • Windows Terminal (when explicitly started)
  • Visual Studio terminal (aka "Developer PowerShell")
  • IntelliJ
  • VSCode

It does not work for:

• conhost
• Windows Terminal when used as default terminal app
  • this one is curious, I wasn't aware of the alternate process structure when starting a process that way

Generally, for all of my use cases, it works. There's of course some room for improvement, but I believe it is significantly better than without the patch, at least.

So, I still vote for merging it, even in the current non-ideal state.

Perhaps we could introduce a better fix by introducing keyboard layout options? For me it would be even better if it just forced the English layout, not looking at the current one I use when processing the shortcuts. But that's a different topic, IMO, and even if we introduce such an option, having a heuristic that makes it work by default in more cases is good anyway.

[iSazonov]

As far as I remember this problem is only with PSRL, yes? I may have missed something, but is it only for Ctrl-C? If so, is it a consequence of setting the ENABLE_PROCESSED_INPUT flag? What if we remove this flag and set the handler to handle Ctrl+C?

[ForNeVeR]

As far as I remember this problem is only with PSRL, yes?

Yep, correct.

I may have missed something, but is it only for Ctrl-C?

No, it is not. Other shortcuts such as Ctrl+R also stop working.

If so, is it a consequence of setting the ENABLE_PROCESSED_INPUT flag?

I've removed the ENABLE_PROCESSED_INPUT flag from the PSReadLine sources (so that it doesn't get passed to SetConsoleInputMode), and it doesn't seem to affect the behavior.

[iSazonov]

It seems I misread ENABLE_PROCESSED_INPUT flag description. I see PSRL clear the flag and system send Ctrl+C to input buffer. So if we set the flag the system will process Ctrl+C and call out callback if we set it. Perhaps this could fix Ctrl+C at least.

[daxian-dbw]

@ForNeVeR Thanks for double verifying all the scenarios. I agree this is the desired improvement even though not perfect. For scenarios where it doesn't work, the old behavior is kept unchanged. I will merge this PR.

@iSazonov It's by design for PSReadLine to clear that flag. When editing, PSReadLine wants to capture ctrl+c and handles it in its own way. The issue here is that ToUnicode treats Ctrl+c as dead key when the Russian keyboard layout is in use. So, another way to see it is: if we can detect dead key in a better way without depending on ToUnicode, then that would the perfect solution to this issue. For now, we are still depending on ToUnicodeEx.