Skip to content

Enable PSReadline to work on NanoServer #3815

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.

Sign up for GitHub

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

Merged
lzybkr merged 4 commits into from
May 31, 2017
Merged

Enable PSReadline to work on NanoServer #3815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61aade8
NanoServer is missing some APIs that PSReadline is using related to f…
May 18, 2017
b7beaba
cache the results of apis not being available
May 22, 2017
f2f808f
fallback to Unix implmentation of KeyChar if ToUnicode() api is not a…
May 23, 2017
c17be88
Revert formatting change
lzybkr May 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Microsoft.PowerShell.PSReadLine/ConsoleKeyChordConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/********************************************************************++
/********************************************************************++
Copyright (c) Microsoft Corporation. All rights reserved.
--********************************************************************/

Expand Down Expand Up @@ -231,7 +231,6 @@ private static bool TryParseCharLiteral(char literal, ref ConsoleModifiers modif
return valid;
}

#if UNIX
// this is borrowed from the CoreFX internal System.IO.StdInReader class
// https://github.com/dotnet/corefx/blob/5b2ae6aa485773cd5569f56f446698633c9ad945/src/System.Console/src/System/IO/StdInReader.cs#L222
private static ConsoleKey GetKeyFromCharValue(char x, out bool isShift, out bool isCtrl)
Expand Down Expand Up @@ -303,7 +302,8 @@ private static ConsoleKey GetKeyFromCharValue(char x, out bool isShift, out bool

return default(ConsoleKey);
}
#else

#if !UNIX
internal static char GetCharFromConsoleKey(ConsoleKey key, ConsoleModifiers modifiers)
{
// default for unprintables and unhandled
Expand Down
47 changes: 39 additions & 8 deletions src/Microsoft.PowerShell.PSReadLine/ConsoleLib.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/********************************************************************++
/********************************************************************++
Copyright (c) Microsoft Corporation. All rights reserved.
--********************************************************************/

Expand Down Expand Up @@ -370,6 +370,23 @@ public struct CHAR_INFO

internal static class ConsoleKeyInfoExtension
{
#if !UNIX
private static bool _toUnicodeApiAvailable = true;

static ConsoleKeyInfoExtension()
{
try
{
var chars = new char[2];
NativeMethods.ToUnicode(13, 28, null, chars, chars.Length, 0);
}
catch (System.EntryPointNotFoundException)
{
_toUnicodeApiAvailable = false; // api not available on NanoServer
}
}
#endif

public static string ToGestureString(this ConsoleKeyInfo key)
{
var mods = key.Modifiers;
Expand All @@ -386,13 +403,16 @@ public static string ToGestureString(this ConsoleKeyInfo key)
sb.Append("Alt");
}

#if UNIX
char c = key.KeyChar;
#else
// Windows cannot use KeyChar as some chords (like Ctrl+[) show up as control characters.
char c = ConsoleKeyChordConverter.GetCharFromConsoleKey(key.Key,
(mods & ConsoleModifiers.Shift) != 0 ? ConsoleModifiers.Shift : 0);
#if !UNIX
if (_toUnicodeApiAvailable)
Copy link
Contributor

@lzybkr lzybkr May 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT May 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@lzybkr lzybkr May 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT May 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT May 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a Nano container that line says:

Ctrl+Oem6             GotoBrace               Go to matching brace

Copy link
Contributor

@lzybkr lzybkr May 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, not very helpful :)

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT May 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@lzybkr lzybkr May 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT May 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
// Windows cannot use KeyChar as some chords (like Ctrl+[) show up as control characters.
c = ConsoleKeyChordConverter.GetCharFromConsoleKey(key.Key,
(mods & ConsoleModifiers.Shift) != 0 ? ConsoleModifiers.Shift : 0);
}
#endif

if (char.IsControl(c) || char.IsWhiteSpace(c))
{
if (key.Modifiers.HasFlag(ConsoleModifiers.Shift))
Expand Down Expand Up @@ -426,6 +446,7 @@ internal class ConhostConsole : IConsole
private bool _istmInitialized = false;
private TEXTMETRIC _tm = new TEXTMETRIC();
private bool _trueTypeInUse = false;
private static bool _getCurrentConsoleFontExApiAvailable = true;

private readonly Lazy<SafeFileHandle> _outputHandle = new Lazy<SafeFileHandle>(() =>
{
Expand Down Expand Up @@ -691,7 +712,18 @@ internal static CONSOLE_FONT_INFO_EX GetConsoleFontInfo(SafeFileHandle consoleHa

CONSOLE_FONT_INFO_EX fontInfo = new CONSOLE_FONT_INFO_EX();
fontInfo.cbSize = Marshal.SizeOf(fontInfo);
bool result = NativeMethods.GetCurrentConsoleFontEx(consoleHandle.DangerousGetHandle(), false, ref fontInfo);
bool result = true;
if (_getCurrentConsoleFontExApiAvailable)
{
try
{
result = NativeMethods.GetCurrentConsoleFontEx(consoleHandle.DangerousGetHandle(), false, ref fontInfo);
}
catch (System.EntryPointNotFoundException)
{
_getCurrentConsoleFontExApiAvailable = false; // api not available on NanoServer
}
}

if (result == false)
{
Expand Down Expand Up @@ -865,7 +897,6 @@ public void StartRender()
CONSOLE_FONT_INFO_EX fontInfo = ConhostConsole.GetConsoleFontInfo(consoleHandle);
int fontType = fontInfo.FontFamily & NativeMethods.FontTypeMask;
_trueTypeInUse = (fontType & NativeMethods.TrueTypeFont) == NativeMethods.TrueTypeFont;

}

public void EndRender()
Expand Down