Skip to content

Changes for Editor Hosts (VSCode/Atom) #626

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
merged 8 commits into from
Jun 5, 2018
Merged

Changes for Editor Hosts (VSCode/Atom) #626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
277e831
Add a cancellation token to ReadLine
SeeminglyScience Jan 25, 2018
537f5f2
Fix stdin locking on Unix platforms
SeeminglyScience Jan 25, 2018
7b9a929
Revert "Fix stdin locking on Unix platforms"
SeeminglyScience Feb 1, 2018
440246a
Addressing feed back
SeeminglyScience Feb 2, 2018
f402081
Change ReadKey method use a overrideable delegate
SeeminglyScience Jun 2, 2018
31853df
Add comment explaining ReadKey private contract
SeeminglyScience Jun 2, 2018
bf930bf
Add a method to force immediate event handling
SeeminglyScience Jun 2, 2018
61e0df0
Add constants for request key wait handle indexes
SeeminglyScience Jun 4, 2018
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
10 changes: 9 additions & 1 deletion PSReadLine/ConsoleLib.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ namespace Microsoft.PowerShell.Internal
{
internal class VirtualTerminal : IConsole
{
// These two fields are used by PowerShellEditorServices to inject a
// custom ReadKey implementation. This is not a public API, but it is
// part of a private contract with that project.
private static Func<bool, ConsoleKeyInfo> _readKeyOverride;

private static Lazy<Func<bool, ConsoleKeyInfo>> _readKeyMethod = new Lazy<Func<bool, ConsoleKeyInfo>>(
() => _readKeyOverride == null ? Console.ReadKey : _readKeyOverride);

public int CursorLeft
{
get => Console.CursorLeft;
Expand Down Expand Up @@ -97,7 +105,7 @@ public Encoding OutputEncoding
set { try { Console.OutputEncoding = value; } catch { } }
}

public ConsoleKeyInfo ReadKey() => Console.ReadKey(true);
public ConsoleKeyInfo ReadKey() => _readKeyMethod.Value(true);
public bool KeyAvailable => Console.KeyAvailable;
public void SetWindowPosition(int left, int top) => Console.SetWindowPosition(left, top);
public void SetCursorPosition(int left, int top) => Console.SetCursorPosition(left, top);
Expand Down
65 changes: 60 additions & 5 deletions PSReadLine/ReadLine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,16 @@ class ExitException : Exception { }

public partial class PSConsoleReadLine : IPSConsoleReadLineMockableMethods
{
private const int ConsoleExiting = 1;

private const int CancellationRequested = 2;

private const int EventProcessingRequested = 3;

private static readonly PSConsoleReadLine _singleton = new PSConsoleReadLine();

private static readonly CancellationToken _defaultCancellationToken = new CancellationTokenSource().Token;

private bool _delayedOneTimeInitCompleted;

private IPSConsoleReadLineMockableMethods _mockableMethods;
Expand All @@ -41,6 +49,8 @@ public partial class PSConsoleReadLine : IPSConsoleReadLineMockableMethods
private Thread _readKeyThread;
private AutoResetEvent _readKeyWaitHandle;
private AutoResetEvent _keyReadWaitHandle;
private AutoResetEvent _forceEventWaitHandle;
private CancellationToken _cancelReadCancellationToken;
internal ManualResetEvent _closingWaitHandle;
private WaitHandle[] _threadProcWaitHandles;
private WaitHandle[] _requestKeyWaitHandles;
Expand Down Expand Up @@ -139,7 +149,12 @@ private void ReadKeyThreadProc()
if (handleId == 1) // It was the _closingWaitHandle that was signaled.
break;

var localCancellationToken = _singleton._cancelReadCancellationToken;
ReadOneOrMoreKeys();
if (localCancellationToken.IsCancellationRequested)
Copy link
Contributor

Choose a reason for hiding this comment

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

If cancellation is requested, what should happen with buffered keys (keys pressed after cancellation, but before the next call to PSReadLIne)?

And what should happen to a partially typed command line (before the cancellation, but no Enter) - should that be restored. If you look at how AcceptAndGetNext is implemented, you can see it's not hard to restore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If cancellation is requested it'll continue to the next loop and any buffered keys will remain in
_queuedKeys until the next key is requested.

Originally any partially typed command lines were just being discarded. Now the
command line is saved and restored when ReadLine is called again. Thanks for pointing out AcceptAndGetNext, I didn't know it would be that easy to implement and it feels much better now.

{
continue;
}

// One or more keys were read - let ReadKey know we're done.
_keyReadWaitHandle.Set();
Expand Down Expand Up @@ -174,9 +189,10 @@ internal static ConsoleKeyInfo ReadKey()
// - a key is pressed
// - the console is exiting
// - 300ms - to process events if we're idle

// - processing of events is requested externally
// - ReadLine cancellation is requested externally
handleId = WaitHandle.WaitAny(_singleton._requestKeyWaitHandles, 300);
if (handleId != WaitHandle.WaitTimeout)
if (handleId != WaitHandle.WaitTimeout && handleId != EventProcessingRequested)
break;

// If we timed out, check for event subscribers (which is just
Expand Down Expand Up @@ -236,7 +252,7 @@ internal static ConsoleKeyInfo ReadKey()
ps?.Dispose();
}

if (handleId == 1)
if (handleId == ConsoleExiting)
{
// The console is exiting - throw an exception to unwind the stack to the point
// where we can return from ReadLine.
Expand All @@ -249,6 +265,18 @@ internal static ConsoleKeyInfo ReadKey()
throw new OperationCanceledException();
}

if (handleId == CancellationRequested)
{
// ReadLine was cancelled. Save the current line to be restored next time ReadLine
// is called, clear the buffer and throw an exception so we can return an empty string.
_singleton.SaveCurrentLine();
_singleton._getNextHistoryIndex = _singleton._history.Count;
_singleton._current = 0;
_singleton._buffer.Clear();
_singleton.Render();
throw new OperationCanceledException();
}

var key = _singleton._queuedKeys.Dequeue();
return key;
}
Expand All @@ -275,6 +303,18 @@ private void PrependQueuedKeys(ConsoleKeyInfo key)
/// </summary>
/// <returns>The complete command line.</returns>
public static string ReadLine(Runspace runspace, EngineIntrinsics engineIntrinsics)
{
// Use a default cancellation token instead of CancellationToken.None because the
// WaitHandle is shared and could be triggered accidently.
return ReadLine(runspace, engineIntrinsics, _defaultCancellationToken);
}

/// <summary>
/// Entry point - called by custom PSHost implementations that require the
/// ability to cancel ReadLine.
/// </summary>
/// <returns>The complete command line.</returns>
public static string ReadLine(Runspace runspace, EngineIntrinsics engineIntrinsics, CancellationToken cancellationToken)
{
var console = _singleton._console;

Expand Down Expand Up @@ -304,11 +344,14 @@ public static string ReadLine(Runspace runspace, EngineIntrinsics engineIntrinsi
_singleton.Initialize(runspace, engineIntrinsics);
}

_singleton._cancelReadCancellationToken = cancellationToken;
_singleton._requestKeyWaitHandles[2] = _singleton._cancelReadCancellationToken.WaitHandle;
return _singleton.InputLoop();
}
catch (OperationCanceledException)
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment below (GitHub won't let me comment on the exact line) is no longer correct with this change - OperationCanceledException can now mean something other than the console is exiting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment has been updated to reflect the new reason for the exception.

{
// Console is exiting - return value isn't too critical - null or 'exit' could work equally well.
// Console is either exiting or the cancellation of ReadLine has been requested
// by a custom PSHost implementation.
return "";
}
catch (ExitException)
Expand Down Expand Up @@ -710,8 +753,9 @@ private void DelayedOneTimeInitialize()

_singleton._readKeyWaitHandle = new AutoResetEvent(false);
_singleton._keyReadWaitHandle = new AutoResetEvent(false);
_singleton._forceEventWaitHandle = new AutoResetEvent(false);
_singleton._closingWaitHandle = new ManualResetEvent(false);
_singleton._requestKeyWaitHandles = new WaitHandle[] {_singleton._keyReadWaitHandle, _singleton._closingWaitHandle};
_singleton._requestKeyWaitHandles = new WaitHandle[] {_singleton._keyReadWaitHandle, _singleton._closingWaitHandle, _defaultCancellationToken.WaitHandle, _singleton._forceEventWaitHandle};
_singleton._threadProcWaitHandles = new WaitHandle[] {_singleton._readKeyWaitHandle, _singleton._closingWaitHandle};

// This is for a "being hosted in an alternate appdomain scenario" (the
Expand All @@ -731,6 +775,17 @@ private void DelayedOneTimeInitialize()
_singleton._readKeyThread.Start();
}

/// <summary>
/// Used by PowerShellEditorServices to force immediate
/// event handling during the <see cref="PSConsoleReadLine.ReadKey" />
/// method. This is not a public API, but it is part of a private contract
/// with that project.
/// </summary>
private static void ForcePSEventHandling()
{
_singleton._forceEventWaitHandle.Set();
}

private static void Chord(ConsoleKeyInfo? key = null, object arg = null)
{
if (!key.HasValue)
Expand Down