From 8c1a0864ec2ca70e49e5b0651a91165bfa3c1435 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sat, 18 Mar 2017 15:23:43 -0700 Subject: [PATCH] Use new strategy for ConsoleReadLine.ReadKeyAsync to fix macOS and Linux issues This change uses a new strategy for providing a quasi-async Console.ReadKey implementation so that key handling works better on Linux and macOS. This should eliminate issues where keys and escape sequences were being echoed incorrectly. It should also eliminate the issue where the backspace key is unusable on these platforms. Resolves PowerShell/vscode-powershell#533. Resolves PowerShell/vscode-powershell#542. --- .../Console/ConsoleReadLine.cs | 594 ++++++++---------- .../Console/ConsoleService.cs | 20 +- .../Session/PowerShellContext.cs | 12 +- .../Session/SessionPSHostUserInterface.cs | 16 +- 4 files changed, 310 insertions(+), 332 deletions(-) diff --git a/src/PowerShellEditorServices/Console/ConsoleReadLine.cs b/src/PowerShellEditorServices/Console/ConsoleReadLine.cs index 267e763ee..ea72000f0 100644 --- a/src/PowerShellEditorServices/Console/ConsoleReadLine.cs +++ b/src/PowerShellEditorServices/Console/ConsoleReadLine.cs @@ -20,6 +20,8 @@ internal class ConsoleReadLine { #region Private Field + private object readKeyLock = new object(); + private ConsoleKeyInfo? bufferedKey; private PowerShellContext powerShellContext; #endregion @@ -48,20 +50,24 @@ public Task ReadSimpleLine(CancellationToken cancellationToken) public async Task ReadSecureLine(CancellationToken cancellationToken) { SecureString secureString = new SecureString(); - ConsoleKeyInfo? typedKey = null; int initialPromptRow = Console.CursorTop; int initialPromptCol = Console.CursorLeft; int previousInputLength = 0; - while (!cancellationToken.IsCancellationRequested) - { - typedKey = await this.ReadKeyAsync(cancellationToken); + Console.TreatControlCAsInput = true; - if (typedKey.HasValue) + try + { + while (!cancellationToken.IsCancellationRequested) { - ConsoleKeyInfo keyInfo = typedKey.Value; + ConsoleKeyInfo keyInfo = await this.ReadKeyAsync(cancellationToken); + if ((int)keyInfo.Key == 3 || + keyInfo.Key == ConsoleKey.C && keyInfo.Modifiers.HasFlag(ConsoleModifiers.Control)) + { + throw new PipelineStoppedException(); + } if (keyInfo.Key == ConsoleKey.Enter) { // Break to return the completed string @@ -78,9 +84,9 @@ public async Task ReadSecureLine(CancellationToken cancellationTok secureString.RemoveAt(secureString.Length - 1); } } - else if (keyInfo.KeyChar != 0) + else if (keyInfo.KeyChar != 0 && !char.IsControl(keyInfo.KeyChar)) { - secureString.AppendChar(typedKey.Value.KeyChar); + secureString.AppendChar(keyInfo.KeyChar); } // Re-render the secure string characters @@ -91,7 +97,7 @@ public async Task ReadSecureLine(CancellationToken cancellationTok { Console.Write('*'); } - else if (previousInputLength > 0) + else if (previousInputLength > 0 && currentInputLength < previousInputLength) { int row = Console.CursorTop, col = Console.CursorLeft; @@ -110,11 +116,10 @@ public async Task ReadSecureLine(CancellationToken cancellationTok previousInputLength = currentInputLength; } - else - { - // Read was cancelled, return null - return null; - } + } + finally + { + Console.TreatControlCAsInput = false; } return secureString; @@ -143,220 +148,263 @@ private async Task ReadLine(bool isCommandLine, CancellationToken cancel int currentCursorIndex = 0; - while (!cancellationToken.IsCancellationRequested) - { - ConsoleKeyInfo? possibleKeyInfo = await this.ReadKeyAsync(cancellationToken); + Console.TreatControlCAsInput = true; - if (!possibleKeyInfo.HasValue) + try + { + while (!cancellationToken.IsCancellationRequested) { - // The read operation was cancelled - return null; - } + ConsoleKeyInfo keyInfo = await this.ReadKeyAsync(cancellationToken); - ConsoleKeyInfo keyInfo = possibleKeyInfo.Value; - - // Do final position calculation after the key has been pressed - // because the window could have been resized before then - int promptStartCol = initialCursorCol; - int promptStartRow = initialCursorRow; - int consoleWidth = Console.WindowWidth; - - // Overwrite any control character if necessary - this.OverwriteControlCharacter( - keyInfo, - inputLine, - promptStartCol, - promptStartRow, - consoleWidth, - currentCursorIndex); + // Do final position calculation after the key has been pressed + // because the window could have been resized before then + int promptStartCol = initialCursorCol; + int promptStartRow = initialCursorRow; + int consoleWidth = Console.WindowWidth; - if (keyInfo.Key == ConsoleKey.Tab && isCommandLine) - { - if (currentCompletion == null) + if ((int)keyInfo.Key == 3 || + keyInfo.Key == ConsoleKey.C && keyInfo.Modifiers.HasFlag(ConsoleModifiers.Control)) { - inputBeforeCompletion = inputLine.ToString(); - inputAfterCompletion = null; - - // TODO: This logic should be moved to AstOperations or similar! - - if (this.powerShellContext.IsDebuggerStopped) + throw new PipelineStoppedException(); + } + else if (keyInfo.Key == ConsoleKey.Tab && isCommandLine) + { + if (currentCompletion == null) { - PSCommand command = new PSCommand(); - command.AddCommand("TabExpansion2"); - command.AddParameter("InputScript", inputBeforeCompletion); - command.AddParameter("CursorColumn", currentCursorIndex); - command.AddParameter("Options", null); + inputBeforeCompletion = inputLine.ToString(); + inputAfterCompletion = null; - var results = - await this.powerShellContext.ExecuteCommand(command, false, false); + // TODO: This logic should be moved to AstOperations or similar! - currentCompletion = results.FirstOrDefault(); - } - else - { - using (RunspaceHandle runspaceHandle = await this.powerShellContext.GetRunspaceHandle()) - using (PowerShell powerShell = PowerShell.Create()) + if (this.powerShellContext.IsDebuggerStopped) { - powerShell.Runspace = runspaceHandle.Runspace; - currentCompletion = - CommandCompletion.CompleteInput( - inputBeforeCompletion, - currentCursorIndex, - null, - powerShell); + PSCommand command = new PSCommand(); + command.AddCommand("TabExpansion2"); + command.AddParameter("InputScript", inputBeforeCompletion); + command.AddParameter("CursorColumn", currentCursorIndex); + command.AddParameter("Options", null); - if (currentCompletion.CompletionMatches.Count > 0) - { - int replacementEndIndex = - currentCompletion.ReplacementIndex + - currentCompletion.ReplacementLength; - - inputAfterCompletion = - inputLine.ToString( - replacementEndIndex, - inputLine.Length - replacementEndIndex); - } - else + var results = + await this.powerShellContext.ExecuteCommand(command, false, false); + + currentCompletion = results.FirstOrDefault(); + } + else + { + using (RunspaceHandle runspaceHandle = await this.powerShellContext.GetRunspaceHandle()) + using (PowerShell powerShell = PowerShell.Create()) { - currentCompletion = null; + powerShell.Runspace = runspaceHandle.Runspace; + currentCompletion = + CommandCompletion.CompleteInput( + inputBeforeCompletion, + currentCursorIndex, + null, + powerShell); + + if (currentCompletion.CompletionMatches.Count > 0) + { + int replacementEndIndex = + currentCompletion.ReplacementIndex + + currentCompletion.ReplacementLength; + + inputAfterCompletion = + inputLine.ToString( + replacementEndIndex, + inputLine.Length - replacementEndIndex); + } + else + { + currentCompletion = null; + } } } } - } - CompletionResult completion = - currentCompletion?.GetNextResult( - !keyInfo.Modifiers.HasFlag(ConsoleModifiers.Shift)); + CompletionResult completion = + currentCompletion?.GetNextResult( + !keyInfo.Modifiers.HasFlag(ConsoleModifiers.Shift)); - if (completion != null) - { - currentCursorIndex = - this.InsertInput( - inputLine, - promptStartCol, - promptStartRow, - $"{completion.CompletionText}{inputAfterCompletion}", - currentCursorIndex, - insertIndex: currentCompletion.ReplacementIndex, - replaceLength: inputLine.Length - currentCompletion.ReplacementIndex, - finalCursorIndex: currentCompletion.ReplacementIndex + completion.CompletionText.Length); + if (completion != null) + { + currentCursorIndex = + this.InsertInput( + inputLine, + promptStartCol, + promptStartRow, + $"{completion.CompletionText}{inputAfterCompletion}", + currentCursorIndex, + insertIndex: currentCompletion.ReplacementIndex, + replaceLength: inputLine.Length - currentCompletion.ReplacementIndex, + finalCursorIndex: currentCompletion.ReplacementIndex + completion.CompletionText.Length); + } } - } - else if (keyInfo.Key == ConsoleKey.LeftArrow) - { - currentCompletion = null; + else if (keyInfo.Key == ConsoleKey.LeftArrow) + { + currentCompletion = null; - if (currentCursorIndex > 0) + if (currentCursorIndex > 0) + { + currentCursorIndex = + this.MoveCursorToIndex( + promptStartCol, + promptStartRow, + consoleWidth, + currentCursorIndex - 1); + } + } + else if (keyInfo.Key == ConsoleKey.Home) { + currentCompletion = null; + currentCursorIndex = this.MoveCursorToIndex( promptStartCol, promptStartRow, consoleWidth, - currentCursorIndex - 1); + 0); } - } - else if (keyInfo.Key == ConsoleKey.Home) - { - currentCompletion = null; - - currentCursorIndex = - this.MoveCursorToIndex( - promptStartCol, - promptStartRow, - consoleWidth, - 0); - } - else if (keyInfo.Key == ConsoleKey.RightArrow) - { - currentCompletion = null; + else if (keyInfo.Key == ConsoleKey.RightArrow) + { + currentCompletion = null; - if (currentCursorIndex < inputLine.Length) + if (currentCursorIndex < inputLine.Length) + { + currentCursorIndex = + this.MoveCursorToIndex( + promptStartCol, + promptStartRow, + consoleWidth, + currentCursorIndex + 1); + } + } + else if (keyInfo.Key == ConsoleKey.End) { + currentCompletion = null; + currentCursorIndex = this.MoveCursorToIndex( promptStartCol, promptStartRow, consoleWidth, - currentCursorIndex + 1); + inputLine.Length); } - } - else if (keyInfo.Key == ConsoleKey.End) - { - currentCompletion = null; - - currentCursorIndex = - this.MoveCursorToIndex( - promptStartCol, - promptStartRow, - consoleWidth, - inputLine.Length); - } - else if (keyInfo.Key == ConsoleKey.UpArrow && isCommandLine) - { - currentCompletion = null; + else if (keyInfo.Key == ConsoleKey.UpArrow && isCommandLine) + { + currentCompletion = null; - // TODO: Ctrl+Up should allow navigation in multi-line input + // TODO: Ctrl+Up should allow navigation in multi-line input - if (currentHistory == null) - { - historyIndex = -1; + if (currentHistory == null) + { + historyIndex = -1; - PSCommand command = new PSCommand(); - command.AddCommand("Get-History"); + PSCommand command = new PSCommand(); + command.AddCommand("Get-History"); - currentHistory = - await this.powerShellContext.ExecuteCommand( - command, - false, - false) as Collection; + currentHistory = + await this.powerShellContext.ExecuteCommand( + command, + false, + false) as Collection; - if (currentHistory != null) + if (currentHistory != null) + { + historyIndex = currentHistory.Count; + } + } + + if (currentHistory != null && currentHistory.Count > 0 && historyIndex > 0) { - historyIndex = currentHistory.Count; + historyIndex--; + + currentCursorIndex = + this.InsertInput( + inputLine, + promptStartCol, + promptStartRow, + (string)currentHistory[historyIndex].Properties["CommandLine"].Value, + currentCursorIndex, + insertIndex: 0, + replaceLength: inputLine.Length); } } + else if (keyInfo.Key == ConsoleKey.DownArrow && isCommandLine) + { + currentCompletion = null; + + // The down arrow shouldn't cause history to be loaded, + // it's only for navigating an active history array - if (currentHistory != null && currentHistory.Count > 0 && historyIndex > 0) + if (historyIndex > -1 && historyIndex < currentHistory.Count && + currentHistory != null && currentHistory.Count > 0) + { + historyIndex++; + + if (historyIndex < currentHistory.Count) + { + currentCursorIndex = + this.InsertInput( + inputLine, + promptStartCol, + promptStartRow, + (string)currentHistory[historyIndex].Properties["CommandLine"].Value, + currentCursorIndex, + insertIndex: 0, + replaceLength: inputLine.Length); + } + else if (historyIndex == currentHistory.Count) + { + currentCursorIndex = + this.InsertInput( + inputLine, + promptStartCol, + promptStartRow, + string.Empty, + currentCursorIndex, + insertIndex: 0, + replaceLength: inputLine.Length); + } + } + } + else if (keyInfo.Key == ConsoleKey.Escape) { - historyIndex--; + currentCompletion = null; + historyIndex = currentHistory != null ? currentHistory.Count : -1; currentCursorIndex = this.InsertInput( inputLine, promptStartCol, promptStartRow, - (string)currentHistory[historyIndex].Properties["CommandLine"].Value, + string.Empty, currentCursorIndex, insertIndex: 0, replaceLength: inputLine.Length); } - } - else if (keyInfo.Key == ConsoleKey.DownArrow && isCommandLine) - { - currentCompletion = null; - - // The down arrow shouldn't cause history to be loaded, - // it's only for navigating an active history array - - if (historyIndex > -1 && historyIndex < currentHistory.Count && - currentHistory != null && currentHistory.Count > 0) + else if (keyInfo.Key == ConsoleKey.Backspace) { - historyIndex++; + currentCompletion = null; - if (historyIndex < currentHistory.Count) + if (currentCursorIndex > 0) { currentCursorIndex = this.InsertInput( inputLine, promptStartCol, promptStartRow, - (string)currentHistory[historyIndex].Properties["CommandLine"].Value, + string.Empty, currentCursorIndex, - insertIndex: 0, - replaceLength: inputLine.Length); + insertIndex: currentCursorIndex - 1, + replaceLength: 1, + finalCursorIndex: currentCursorIndex - 1); } - else if (historyIndex == currentHistory.Count) + } + else if (keyInfo.Key == ConsoleKey.Delete) + { + currentCompletion = null; + + if (currentCursorIndex < inputLine.Length) { currentCursorIndex = this.InsertInput( @@ -365,118 +413,88 @@ await this.powerShellContext.ExecuteCommand( promptStartRow, string.Empty, currentCursorIndex, - insertIndex: 0, - replaceLength: inputLine.Length); + replaceLength: 1, + finalCursorIndex: currentCursorIndex); } } - } - else if (keyInfo.Key == ConsoleKey.Escape) - { - currentCompletion = null; - historyIndex = currentHistory != null ? currentHistory.Count : -1; - - currentCursorIndex = - this.InsertInput( - inputLine, - promptStartCol, - promptStartRow, - string.Empty, - currentCursorIndex, - insertIndex: 0, - replaceLength: inputLine.Length); - } - else if (keyInfo.Key == ConsoleKey.Backspace) - { - currentCompletion = null; - - if (currentCursorIndex > 0) + else if (keyInfo.Key == ConsoleKey.Enter) { - currentCursorIndex = - this.InsertInput( - inputLine, - promptStartCol, - promptStartRow, - string.Empty, - currentCursorIndex, - insertIndex: currentCursorIndex - 1, - replaceLength: 1, - finalCursorIndex: currentCursorIndex - 1); + string completedInput = inputLine.ToString(); + currentCompletion = null; + currentHistory = null; + + //if ((keyInfo.Modifiers & ConsoleModifiers.Shift) == ConsoleModifiers.Shift) + //{ + // // TODO: Start a new line! + // continue; + //} + + Parser.ParseInput( + completedInput, + out Token[] tokens, + out ParseError[] parseErrors); + + //if (parseErrors.Any(e => e.IncompleteInput)) + //{ + // // TODO: Start a new line! + // continue; + //} + + return completedInput; } - } - else if (keyInfo.Key == ConsoleKey.Delete) - { - currentCompletion = null; - - if (currentCursorIndex < inputLine.Length) + else if (keyInfo.KeyChar != 0 && !char.IsControl(keyInfo.KeyChar)) { + // Normal character input + currentCompletion = null; + currentCursorIndex = this.InsertInput( inputLine, promptStartCol, promptStartRow, - string.Empty, + keyInfo.KeyChar.ToString(), currentCursorIndex, - replaceLength: 1, - finalCursorIndex: currentCursorIndex); + finalCursorIndex: currentCursorIndex + 1); } } - else if (keyInfo.Key == ConsoleKey.Enter) - { - string completedInput = inputLine.ToString(); - currentCompletion = null; - currentHistory = null; - - //if ((keyInfo.Modifiers & ConsoleModifiers.Shift) == ConsoleModifiers.Shift) - //{ - // // TODO: Start a new line! - // continue; - //} - - Parser.ParseInput( - completedInput, - out Token[] tokens, - out ParseError[] parseErrors); - - //if (parseErrors.Any(e => e.IncompleteInput)) - //{ - // // TODO: Start a new line! - // continue; - //} - - return completedInput; - } - else if (keyInfo.KeyChar != 0) - { - // Normal character input - currentCompletion = null; - - currentCursorIndex = - this.InsertInput( - inputLine, - promptStartCol, - promptStartRow, - keyInfo.KeyChar.ToString(), - currentCursorIndex, - finalCursorIndex: currentCursorIndex + 1); - } + } + finally + { + Console.TreatControlCAsInput = false; } return null; } - private async Task ReadKeyAsync(CancellationToken cancellationToken) + private async Task ReadKeyAsync(CancellationToken cancellationToken) { - while (!cancellationToken.IsCancellationRequested) - { - if (Console.KeyAvailable) - { - return Console.ReadKey(true); - } + return await + Task.Factory.StartNew( + () => + { + ConsoleKeyInfo keyInfo; - await Task.Delay(25); - } + lock (this.readKeyLock) + { + if (this.bufferedKey.HasValue) + { + keyInfo = this.bufferedKey.Value; + this.bufferedKey = null; + } + else + { + keyInfo = Console.ReadKey(true); - return null; + if (cancellationToken.IsCancellationRequested) + { + this.bufferedKey = keyInfo; + throw new TaskCanceledException(); + } + } + } + + return keyInfo; + }); } private int CalculateIndexFromCursor( @@ -601,70 +619,6 @@ private int MoveCursorToIndex( return newCursorIndex; } - private void OverwriteControlCharacter( - ConsoleKeyInfo keyInfo, - StringBuilder inputLine, - int promptStartCol, - int promptStartRow, - int consoleWidth, - int cursorIndex) - { - bool overwriteNeeded = false; - - switch (keyInfo.Key) - { - case ConsoleKey.LeftArrow: - case ConsoleKey.RightArrow: - case ConsoleKey.Delete: - case ConsoleKey.Backspace: - case ConsoleKey.Home: - case ConsoleKey.End: - case ConsoleKey.Escape: - case ConsoleKey.Tab: - overwriteNeeded = true; - break; - } - - if (overwriteNeeded) - { - // TODO: Adjust this based on actual escape char length - const int overwriteCount = 5; - - this.CalculateCursorFromIndex( - promptStartCol, - promptStartRow, - consoleWidth, - cursorIndex, - out int cursorCol, - out int cursorRow); - - Console.SetCursorPosition(cursorCol, cursorRow); - - // Calculate the index of the input line to overwrite - // plus any extra padding that's needed to cover characters - // that extend outside the length of the input string - int overwriteIndex = cursorIndex + overwriteCount; - int padLength = Math.Max(0, overwriteIndex - inputLine.Length); - if (padLength > 0) - { - overwriteIndex -= padLength; - } - - // Re-render affected input string and padding characters - Console.Write( - inputLine.ToString( - cursorIndex, - overwriteIndex - cursorIndex)); - - Console.Write( - new string( - ' ', - padLength)); - - Console.SetCursorPosition(cursorCol, cursorRow); - } - } - #endregion } } diff --git a/src/PowerShellEditorServices/Console/ConsoleService.cs b/src/PowerShellEditorServices/Console/ConsoleService.cs index 5efa1a4d3..3a14bd956 100644 --- a/src/PowerShellEditorServices/Console/ConsoleService.cs +++ b/src/PowerShellEditorServices/Console/ConsoleService.cs @@ -114,14 +114,8 @@ public void StartReadLoop() Task.Factory.StartNew( async () => { - // Set the thread's name to help with debugging - Thread.CurrentThread.Name = "Terminal Input Loop Thread"; - await this.StartReplLoop(this.readLineCancellationToken.Token); - }, - CancellationToken.None, - TaskCreationOptions.LongRunning, - TaskScheduler.Default); + }); } else { @@ -314,6 +308,18 @@ private async Task StartReplLoop(CancellationToken cancellationToken) await this.consoleReadLine.ReadCommandLine( cancellationToken); } + catch (PipelineStoppedException) + { + this.WriteOutput( + "^C", + true, + OutputType.Normal, + foregroundColor: ConsoleColor.Red); + } + catch (TaskCanceledException) + { + // Do nothing here, the while loop condition will exit. + } catch (Exception e) // Narrow this if possible { this.WriteOutput( diff --git a/src/PowerShellEditorServices/Session/PowerShellContext.cs b/src/PowerShellEditorServices/Session/PowerShellContext.cs index 101cfee09..c3e4c8966 100644 --- a/src/PowerShellEditorServices/Session/PowerShellContext.cs +++ b/src/PowerShellEditorServices/Session/PowerShellContext.cs @@ -23,7 +23,6 @@ namespace Microsoft.PowerShell.EditorServices using System.Management.Automation.Runspaces; using Microsoft.PowerShell.EditorServices.Session.Capabilities; using System.IO; - using System.Security; /// /// Manages the lifetime and usage of a PowerShell session. @@ -847,11 +846,18 @@ internal void ResumeDebugger(DebuggerResumeAction resumeAction) { // Set the result so that the execution thread resumes. // The execution thread will clean up the task. - this.debuggerStoppedTask.SetResult(resumeAction); + if (!this.debuggerStoppedTask.TrySetResult(resumeAction)) + { + Logger.Write( + LogLevel.Error, + $"Tried to resume debugger with action {resumeAction} but the task was already completed."); + } } else { - // TODO: Throw InvalidOperationException? + Logger.Write( + LogLevel.Error, + $"Tried to resume debugger with action {resumeAction} but there was no debuggerStoppedTask."); } } diff --git a/src/PowerShellEditorServices/Session/SessionPSHostUserInterface.cs b/src/PowerShellEditorServices/Session/SessionPSHostUserInterface.cs index 0c6627e81..17ba19adc 100644 --- a/src/PowerShellEditorServices/Session/SessionPSHostUserInterface.cs +++ b/src/PowerShellEditorServices/Session/SessionPSHostUserInterface.cs @@ -471,12 +471,24 @@ private void WaitForPromptCompletion( } catch (AggregateException e) { + // Find the right InnerException + Exception innerException = e.InnerException; + if (innerException is AggregateException) + { + innerException = innerException.InnerException; + } + // Was the task cancelled? - if (e.InnerExceptions[0] is TaskCanceledException) + if (innerException is TaskCanceledException) { // Stop the pipeline if the prompt was cancelled throw new PipelineStoppedException(); } + else if (innerException is PipelineStoppedException) + { + // The prompt is being cancelled, rethrow the exception + throw innerException; + } else { // Rethrow the exception @@ -484,7 +496,7 @@ private void WaitForPromptCompletion( string.Format( "{0} failed, check inner exception for details", promptFunctionName), - e.InnerException); + innerException); } } }