Enable PSReadline to work on NanoServer #3815

SteveL-MSFT · 2017-05-18T21:30:05Z

PSReadline uses two native Win32 apis to determine font and unicode information which is not available on NanoServer. Fix is to assume the input is one byte in character in size and not using a truetype font. This should work for most use cases.

Manually tested this change with our published dockerfile for NanoServer.

Fix #3463

…ont and unicode which causes PSReadline to not be usable on NanoServer docker images. Fix is to assume single byte characters if these APIs are not available.

lzybkr · 2017-05-22T04:17:05Z

src/Microsoft.PowerShell.PSReadLine/ConsoleKeyChordConverter.cs

+            int charCount = 1;
+            try
+            {
+                charCount = NativeMethods.ToUnicode(


We should find a better way - this will cause an exception on every character typed, not good for performance and extremely annoying when debugging.

There is some one-time initialization code - so one less ugly option is to add a P/Invoke call there just to detect if the api is present or not, setting a flag that is checked here.

Will make the change

I looked into putting the check in Readline.Initialize() but since it was a different class I would have to expose two additional members. Seemed simpler to keep the state within the classes needing those apis.

lzybkr · 2017-05-22T04:17:35Z

src/Microsoft.PowerShell.PSReadLine/ConsoleLib.cs

+            bool result = true;
+            try
+            {
+                result = NativeMethods.GetCurrentConsoleFontEx(consoleHandle.DangerousGetHandle(), false, ref fontInfo);


We should do the same here - detect once so we don't have frequent exceptions.

Will change

SteveL-MSFT · 2017-05-22T23:32:28Z

@lzybkr fixed

lzybkr · 2017-05-23T18:56:57Z

src/Microsoft.PowerShell.PSReadLine/ConsoleKeyChordConverter.cs

-            int charCount = NativeMethods.ToUnicode(
-                virtualKey, scanCode, state, chars, chars.Length, NativeMethods.MENU_IS_INACTIVE);
+            int charCount = 1;
+            if (_toUnicodeApiAvailable)


After a closer look, I don't think this is the right fix for this case - custom key bindings need a valid results, you'll just return \0.

I think we'll need the Unix variant of this code as a fallback, or potentially use it unconditionally, though that might break some more obscure key bindings.

There's a comment in the code that GetCharFromConsoleKey() is needed to support chords like Ctrl+[. Seems like the best approach here is to defer to the Unix code if this API isn't available. I'll make that change.

SteveL-MSFT · 2017-05-23T21:02:59Z

Manually verified, you can use something like this:

docker run -it --entrypoint cmd -v C:\users\slee\repos\PowerShell\src\powershell-win-core\bin\Debug\netcoreapp2.0\win10-x64\publish:c:\powershell microsoft/powershell:nanoserver

After the container starts in cmd, you can start powershell using c:\powershell\powershell

…vailable (such as NanoServer)

SteveL-MSFT · 2017-05-26T03:55:29Z

@lzybkr good to go?

lzybkr · 2017-05-26T05:22:26Z

src/Microsoft.PowerShell.PSReadLine/ConsoleLib.cs

-            char c = ConsoleKeyChordConverter.GetCharFromConsoleKey(key.Key,
-                (mods & ConsoleModifiers.Shift) != 0 ? ConsoleModifiers.Shift : 0);
+#if !UNIX
+            if (_toUnicodeApiAvailable)


If this api is not available, based on the comment, I would expect to see some garbage in the output of Get-PSReadlineKeyHandler -Bound, e.g. on this line:

Ctrl+] GotoBrace Go to matching brace

(or if you are using Emacs mode):

Ctrl+] CharacterSearch Read a character and move the cursor to the next occurence of that character Ctrl+Alt+] CharacterSearchBackward Read a character and move the cursor to the previous occurence of that char...

I can check next week when back in office where I have Docker and a Nano container, but not sure what you are proposing here when the api is not available.

I'm proposing not degrading the default user experience if possible - which might require a few special cases. I would guess a more general fix isn't possible because this api is that general fix.

Seems like this is a decision the Nano team should make. If they want the full experience, they need those apis on Nano. I'll certainly let them know of the limitation.

On a Nano container that line says:

Ctrl+Oem6 GotoBrace Go to matching brace

Indeed, not very helpful :)

If you want, I can add specialized code so that if those apis aren't available, we don't have those bindings by default. Seems like over optimizing for what is today an edge case. Alternatively, I could have PSReadline fail to load if those apis aren't available even though most of the PSReadline capability seems to run fine on Nano. Seems like the best option is to document this and have Nano team make the call to add that api.

A few special cases is not over optimizing and is a practical assuming those apis aren't really generally useful.
The default output of Get-PSReadlineKeyBindings should not contain Oem.

I created #3890 to track this issue, so we can at least unblock Nano for now.

NanoServer is missing some APIs that PSReadline is using related to f…

61aade8

…ont and unicode which causes PSReadline to not be usable on NanoServer docker images. Fix is to assume single byte characters if these APIs are not available.

SteveL-MSFT assigned lzybkr May 18, 2017

SteveL-MSFT requested a review from lzybkr May 18, 2017 21:30

msftclas added the cla-not-required label May 18, 2017

lzybkr suggested changes May 22, 2017

View reviewed changes

cache the results of apis not being available

b7beaba

lzybkr reviewed May 23, 2017

View reviewed changes

SteveL-MSFT force-pushed the psreadline-docker branch 5 times, most recently from e983193 to 3f005a5 Compare May 24, 2017 01:58

fallback to Unix implmentation of KeyChar if ToUnicode() api is not a…

f2f808f

…vailable (such as NanoServer)

SteveL-MSFT force-pushed the psreadline-docker branch from 3f005a5 to f2f808f Compare May 24, 2017 21:31

Revert formatting change

c17be88

lzybkr reviewed May 26, 2017

View reviewed changes

SteveL-MSFT mentioned this pull request May 30, 2017

PSReadline should not bind to keys that are not usable #3890

Closed

lzybkr approved these changes May 31, 2017

View reviewed changes

lzybkr merged commit ee45650 into PowerShell:master May 31, 2017

SteveL-MSFT deleted the psreadline-docker branch May 31, 2017 17:27

SteveL-MSFT mentioned this pull request Jul 19, 2017

PowerShell v6.0.0-alpha.14 is crashing in Container interactive terminals PowerShell/PSReadLine#492

Closed

Enable PSReadline to work on NanoServer #3815

Enable PSReadline to work on NanoServer #3815

Uh oh!

Conversation

SteveL-MSFT commented May 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveL-MSFT commented May 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveL-MSFT commented May 23, 2017

Uh oh!

SteveL-MSFT commented May 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SteveL-MSFT commented May 18, 2017 •

edited

Loading