Skip to content

Add private contract delegate for PSES to handle idle on readline #1679

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

Merged
merged 7 commits into from
Oct 27, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Cleanup updates
  • Loading branch information
daxian-dbw committed Oct 27, 2021
commit 3d556aaed1ac2f377f91b4289226cf40d1c4a580
2 changes: 1 addition & 1 deletion PSReadLine.build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ task LayoutModule BuildPolyfiller, BuildMainModule, {
Copy-Item $binPath/Microsoft.PowerShell.Pager.dll $targetDir

if ($Configuration -eq 'Debug') {
Copy-Item $binPath/*.pdb $targetDir
Copy-Item $binPath/*.pdb $targetDir
}

if (Test-Path $binPath/System.Runtime.InteropServices.RuntimeInformation.dll) {
Expand Down
22 changes: 6 additions & 16 deletions PSReadLine/ReadLine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,15 @@ public partial class PSConsoleReadLine : IPSConsoleReadLineMockableMethods

private const int CancellationRequested = 2;

private const int EventProcessingRequested = 3;

// *must* be initialized in the static ctor
// because the static member _clipboard depends upon it
// for its own initialization
private static readonly PSConsoleReadLine _singleton;

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

// This exists for PowerShell Editor Services (the backend of the PowerShell VSCode extension)
// so that it can call PSReadLine from a delegate and not hit nested pipeline issues
// This is used by PowerShellEditorServices (the backend of the PowerShell VSCode extension)
// so that it can call PSReadLine from a delegate and not hit nested pipeline issues.
private static Action<CancellationToken> _handleIdleOverride;

private bool _delayedOneTimeInitCompleted;
Expand All @@ -59,7 +57,6 @@ 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;
Expand Down Expand Up @@ -199,11 +196,13 @@ internal static PSKeyInfo ReadKey()
// Next, wait for one of three things:
// - a key is pressed
// - the console is exiting
// - 300ms - to process events if we're idle
// - 300ms timeout - to process events if we're idle
// - ReadLine cancellation is requested externally
handleId = WaitHandle.WaitAny(_singleton._requestKeyWaitHandles, 300);
if (handleId != WaitHandle.WaitTimeout && handleId != EventProcessingRequested)
if (handleId != WaitHandle.WaitTimeout)
{
break;
}

if (_handleIdleOverride is not null)
Copy link
Member

Choose a reason for hiding this comment

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

By removing _forceEventWaitHandle, how do you break out WaitHandle.WaitAny immediately when you need to? Are you planning to use the cancellation token for that purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we just cancel in that scenario. It's not perfect because of how PSRL handles cancellation (i.e. it returns a value rather than throwing an OperationCanceledException) but because we know we cancelled, we can take the appropriate action.

Copy link
Member

Choose a reason for hiding this comment

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

Chatted with @rjmholt offline, and we decided to remove the _forceEventWaitHandle for now as it's not needed in the current PSES pipeline implementation. I will add it back if it turns out to be required in future.

{
Expand Down Expand Up @@ -319,15 +318,6 @@ public static string ReadLine(Runspace runspace, EngineIntrinsics engineIntrinsi
return ReadLine(runspace, engineIntrinsics, _defaultCancellationToken, lastRunStatus);
}

/// <summary>
/// Temporary entry point for PowerShell VSCode extension to avoid breaking the existing PSES.
/// PSES will need to move away from this entry point to actually provide information about 'lastRunStatus'.
/// </summary>
public static string ReadLine(Runspace runspace, EngineIntrinsics engineIntrinsics, CancellationToken cancellationToken)
{
return ReadLine(runspace, engineIntrinsics, cancellationToken, lastRunStatus: null);
}

/// <summary>
/// Entry point - called by custom PSHost implementations that require the
/// ability to cancel ReadLine.
Expand Down