Skip to content

Added a -UseHostReadKey switch to instruct LegacyReadLine to use the … #1748

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 7 additions & 4 deletions module/PowerShellEditorServices/Start-EditorServices.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,20 @@ param(
[ValidateSet("Diagnostic", "Verbose", "Normal", "Warning", "Error")]
$LogLevel,

[Parameter(Mandatory=$true)]
[ValidateNotNullOrEmpty()]
[string]
$SessionDetailsPath,
[Parameter(Mandatory=$true)]
[ValidateNotNullOrEmpty()]
[string]
$SessionDetailsPath,

[switch]
$EnableConsoleRepl,

[switch]
$UseLegacyReadLine,

[switch]
$UseHostReadKey,

[switch]
$DebugServiceOnly,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,12 @@ public StartEditorServicesCommand()
[Parameter]
public SwitchParameter UseLegacyReadLine { get; set; }

/// <summary>
/// When set and the console is enabled and legacy readline
/// is enabled, console operations will use PSHostreadline implementation will be used instead of PSReadLine.
/// </summary>
[Parameter]
public SwitchParameter UseHostReadKey { get; set; }
/// <summary>
/// When set, do not enable LSP service, only the debug adapter.
/// </summary>
Expand Down Expand Up @@ -345,8 +351,9 @@ private EditorServicesConfig CreateConfigObject()
hostInfo,
Host,
SessionDetailsPath,
bundledModulesPath,
LogPath)
bundledModulesPath,
LogPath,
UseHostReadKey)
{
FeatureFlags = FeatureFlags,
LogLevel = LogLevel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public enum ConsoleReplKind
/// <summary>Use a REPL with the legacy readline implementation. This is generally used when PSReadLine is unavailable.</summary>
LegacyReadLine = 1,
/// <summary>Use a REPL with the PSReadLine module for console interaction.</summary>
PSReadLine = 2,
PSReadLine = 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
PSReadLine = 2
PSReadLine = 2,

Trailing comma is on purpose here

}

/// <summary>
Expand All @@ -39,13 +39,15 @@ public EditorServicesConfig(
PSHost psHost,
string sessionDetailsPath,
string bundledModulePath,
string logPath)
string logPath,
bool useHostReadKey)
{
HostInfo = hostInfo;
PSHost = psHost;
SessionDetailsPath = sessionDetailsPath;
BundledModulePath = bundledModulePath;
LogPath = logPath;
UseHostReadKey = useHostReadKey;
}

/// <summary>
Expand All @@ -72,6 +74,7 @@ public EditorServicesConfig(
/// The path to use for logging for Editor Services.
/// </summary>
public string LogPath { get; }
public bool UseHostReadKey { get; }

/// <summary>
/// Names of or paths to any additional modules to load on startup.
Expand All @@ -88,7 +91,7 @@ public EditorServicesConfig(
/// (including none to disable the integrated console).
/// </summary>
public ConsoleReplKind ConsoleRepl { get; set; } = ConsoleReplKind.None;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Extra whitespace

/// <summary>
/// The minimum log level to log events with.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public static EditorServicesLoader Create(

string asmPath = Path.Combine(s_psesDependencyDirPath, $"{asmName.Name}.dll");

logger.Log(PsesLogLevel.Verbose, "Loading PSES DLL using new assembly load context");
logger.Log(PsesLogLevel.Verbose, $"Loading {asmName.Name}.dll using new assembly load context");

return psesLoadContext.LoadFromAssemblyPath(asmPath);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ private HostStartupInfo CreateHostStartupInfo()
(int)_config.LogLevel,
consoleReplEnabled: _config.ConsoleRepl != ConsoleReplKind.None,
usesLegacyReadLine: _config.ConsoleRepl == ConsoleReplKind.LegacyReadLine,
useHostReadKey: _config.UseHostReadKey,
bundledModulePath: _config.BundledModulePath);
}

Expand Down
8 changes: 7 additions & 1 deletion src/PowerShellEditorServices/Hosting/HostStartupInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ public sealed class HostStartupInfo
/// If the console REPL is not enabled, this setting will be ignored.
/// </summary>
public bool UsesLegacyReadLine { get; }

/// <summary>
/// If true, the legacy PSES readline implementation but calls ReadKey in the provided host
/// If the console REPL is not enabled, this setting will be ignored.
/// </summary>
public bool UseHostReadKey { get; }
/// <summary>
/// The PowerShell host to use with Editor Services.
/// </summary>
Expand Down Expand Up @@ -153,6 +157,7 @@ public HostStartupInfo(
int logLevel,
bool consoleReplEnabled,
bool usesLegacyReadLine,
bool useHostReadKey,
string bundledModulePath)
{
Name = name ?? DefaultHostName;
Expand All @@ -167,6 +172,7 @@ public HostStartupInfo(
LogLevel = logLevel;
ConsoleReplEnabled = consoleReplEnabled;
UsesLegacyReadLine = usesLegacyReadLine;
UseHostReadKey = useHostReadKey;
BundledModulePath = bundledModulePath;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public override string ReadLine(CancellationToken cancellationToken)
// because the window could have been resized before then
int promptStartCol = initialCursorCol;
int promptStartRow = initialCursorRow;

int consoleWidth = Console.WindowWidth;

switch (keyInfo.Key)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ internal class PsrlReadLine : TerminalReadLine
private readonly PsesInternalHost _psesHost;

private readonly EngineIntrinsics _engineIntrinsics;


public PsrlReadLine(
PSReadLineProxy psrlProxy,
Expand All @@ -29,7 +30,7 @@ public PsrlReadLine(
_psesHost = psesHost;
_engineIntrinsics = engineIntrinsics;
_psrlProxy.OverrideReadKey(readKeyFunc);
_psrlProxy.OverrideIdleHandler(onIdleAction);
_psrlProxy.OverrideIdleHandler(onIdleAction);
}

public override string ReadLine(CancellationToken cancellationToken) => _psesHost.InvokeDelegate(
Expand All @@ -39,7 +40,7 @@ public override string ReadLine(CancellationToken cancellationToken) => _psesHos
cancellationToken);

protected override ConsoleKeyInfo ReadKey(CancellationToken cancellationToken) => _psesHost.ReadKey(intercept: true, cancellationToken);

private string InvokePSReadLine(CancellationToken cancellationToken)
{
EngineIntrinsics engineIntrinsics = _psesHost.IsRunspacePushed ? null : _engineIntrinsics;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public PsesInternalHost(
_logger = loggerFactory.CreateLogger<PsesInternalHost>();
_languageServer = languageServer;
_hostInfo = hostInfo;


// Respect a user provided bundled module path.
if (Directory.Exists(hostInfo.BundledModulePath))
Expand Down Expand Up @@ -136,6 +137,7 @@ public PsesInternalHost(
Version = hostInfo.Version;

DebugContext = new PowerShellDebugContext(loggerFactory, this);

UI = hostInfo.ConsoleReplEnabled
? new EditorServicesConsolePSHostUserInterface(loggerFactory, hostInfo.PSHost.UI)
: new NullPSHostUI();
Expand Down Expand Up @@ -1071,6 +1073,24 @@ private void OnCancelKeyPress(object sender, ConsoleCancelEventArgs args)
StopDebugContext();
}
}
/// <summary>
/// This method is sent to PSReadLine as a workaround for issues with the System.Console
/// implementation. Functionally it is the same as System.Console.ReadKey,
/// with the exception that it will not lock the standard input stream.
/// </summary>
/// <param name="intercept">
/// Determines whether to display the pressed key in the console window.
/// true to not display the pressed key; otherwise, false.
/// </param>
/// <param name="cancellationToken">
/// The <see cref="CancellationToken" /> that can be used to cancel the request.
/// </param>
/// <returns>
/// An object that describes the ConsoleKey constant and Unicode character, if any,
/// that correspond to the pressed console key. The ConsoleKeyInfo object also describes,
/// in a bitwise combination of ConsoleModifiers values, whether one or more Shift, Alt,
/// or Ctrl modifier keys was pressed simultaneously with the console key.
/// </returns>

private ConsoleKeyInfo ReadKey(bool intercept)
{
Expand All @@ -1094,13 +1114,24 @@ private ConsoleKeyInfo ReadKey(bool intercept)
// we can subscribe in the same way.
DebugServer?.SendNotification("powerShell/sendKeyPress");
});

// PSReadLine doesn't tell us when CtrlC was sent. So instead we keep track of the last
// key here. This isn't functionally required, but helps us determine when the prompt
// needs a newline added
//
// TODO: We may want to allow users of PSES to override this method call.
_lastKey = System.Console.ReadKey(intercept);
if (!_hostInfo.UseHostReadKey)
{
_lastKey = System.Console.ReadKey(intercept);
}
else
{
KeyInfo keyInfo = _hostInfo.PSHost.UI.RawUI.ReadKey();
Copy link
Member

Choose a reason for hiding this comment

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

Silly question: Could we just make the default RawUI.ReadKey in PSES be Console.ReadKey()? I ask because I see that's how you're supplying a custom ReadKey, which is exactly what we're also trying to do.

_lastKey = new(keyInfo.Character, (ConsoleKey)keyInfo.Character, (keyInfo.ControlKeyState & ControlKeyStates.ShiftPressed) > 0,
(keyInfo.ControlKeyState & (ControlKeyStates.RightAltPressed | ControlKeyStates.LeftAltPressed)) > 0,
(keyInfo.ControlKeyState & (ControlKeyStates.RightCtrlPressed | ControlKeyStates.LeftCtrlPressed)) > 0);
}

return _lastKey.Value;
}

Expand Down
1 change: 1 addition & 0 deletions test/PowerShellEditorServices.Test/PsesHostFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public static PsesInternalHost Create(ILoggerFactory loggerFactory)
logLevel: (int)LogLevel.None,
consoleReplEnabled: false,
usesLegacyReadLine: false,
useHostReadKey: false,
bundledModulePath: BundledModulePath);

PsesInternalHost psesHost = new(loggerFactory, null, testHostDetails);
Expand Down