-
Notifications
You must be signed in to change notification settings - Fork 237
PSReadLine integration #672
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
Changes from 19 commits
5a6eba6
1930afe
7e26e4e
ac44055
d2e1ceb
a507705
a870ee2
190cc0c
49db2ba
379eee4
e16c823
cc62dab
7f2b5b8
e19afe6
afdfb43
3575c79
6a3f7c9
cc10b91
86ab115
b51cc75
1682410
d68fb70
2968d1f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
// | ||
// Copyright (c) Microsoft. All rights reserved. | ||
// Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
// | ||
|
||
using System; | ||
using System.Runtime.InteropServices; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
namespace Microsoft.PowerShell.EditorServices.Console | ||
{ | ||
/// <summary> | ||
/// Provides asynchronous implementations of the <see cref="Console" /> API's as well as | ||
/// synchronous implementations that work around platform specific issues. | ||
/// </summary> | ||
internal static class ConsoleProxy | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A comment to describe what the class is and what it does could be very helpful in the future |
||
{ | ||
private static IConsoleOperations s_consoleProxy; | ||
|
||
static ConsoleProxy() | ||
{ | ||
// Maybe we should just include the RuntimeInformation package for FullCLR? | ||
#if CoreCLR | ||
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) | ||
{ | ||
s_consoleProxy = new WindowsConsoleOperations(); | ||
return; | ||
} | ||
|
||
s_consoleProxy = new UnixConsoleOperations(); | ||
#else | ||
s_consoleProxy = new WindowsConsoleOperations(); | ||
#endif | ||
} | ||
|
||
public static Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken cancellationToken) => | ||
s_consoleProxy.ReadKeyAsync(cancellationToken); | ||
|
||
public static int GetCursorLeft() => | ||
s_consoleProxy.GetCursorLeft(); | ||
|
||
public static int GetCursorLeft(CancellationToken cancellationToken) => | ||
s_consoleProxy.GetCursorLeft(cancellationToken); | ||
|
||
public static Task<int> GetCursorLeftAsync() => | ||
s_consoleProxy.GetCursorLeftAsync(); | ||
|
||
public static Task<int> GetCursorLeftAsync(CancellationToken cancellationToken) => | ||
s_consoleProxy.GetCursorLeftAsync(cancellationToken); | ||
|
||
public static int GetCursorTop() => | ||
s_consoleProxy.GetCursorTop(); | ||
|
||
public static int GetCursorTop(CancellationToken cancellationToken) => | ||
s_consoleProxy.GetCursorTop(cancellationToken); | ||
|
||
public static Task<int> GetCursorTopAsync() => | ||
s_consoleProxy.GetCursorTopAsync(); | ||
|
||
public static Task<int> GetCursorTopAsync(CancellationToken cancellationToken) => | ||
s_consoleProxy.GetCursorTopAsync(cancellationToken); | ||
|
||
/// <summary> | ||
/// On Unix platforms this method is sent to PSReadLine as a work around for issues | ||
/// with the System.Console implementation for that platform. 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: keys were pressed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can change it, but it's from the docs for Console.ReadKey. |
||
/// </returns> | ||
internal static ConsoleKeyInfo UnixReadKey(bool intercept, CancellationToken cancellationToken) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is really nice! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! Admitably most of that comes from System.Console.ReadKey, so I can't take much credit there 😉 |
||
{ | ||
try | ||
{ | ||
return ((UnixConsoleOperations)s_consoleProxy).ReadKey(intercept, cancellationToken); | ||
} | ||
catch (OperationCanceledException) | ||
{ | ||
return default(ConsoleKeyInfo); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,6 @@ | |
using System.Collections.ObjectModel; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Runtime.InteropServices; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
|
@@ -20,27 +19,13 @@ namespace Microsoft.PowerShell.EditorServices.Console | |
internal class ConsoleReadLine | ||
{ | ||
#region Private Field | ||
private static IConsoleOperations s_consoleProxy; | ||
|
||
private PowerShellContext powerShellContext; | ||
|
||
#endregion | ||
|
||
#region Constructors | ||
static ConsoleReadLine() | ||
{ | ||
// Maybe we should just include the RuntimeInformation package for FullCLR? | ||
#if CoreCLR | ||
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) | ||
{ | ||
s_consoleProxy = new WindowsConsoleOperations(); | ||
return; | ||
} | ||
|
||
s_consoleProxy = new UnixConsoleOperations(); | ||
#else | ||
s_consoleProxy = new WindowsConsoleOperations(); | ||
#endif | ||
} | ||
|
||
public ConsoleReadLine(PowerShellContext powerShellContext) | ||
|
@@ -66,8 +51,8 @@ public async Task<SecureString> ReadSecureLine(CancellationToken cancellationTok | |
{ | ||
SecureString secureString = new SecureString(); | ||
|
||
int initialPromptRow = Console.CursorTop; | ||
int initialPromptCol = Console.CursorLeft; | ||
int initialPromptRow = await ConsoleProxy.GetCursorTopAsync(cancellationToken); | ||
int initialPromptCol = await ConsoleProxy.GetCursorLeftAsync(cancellationToken); | ||
int previousInputLength = 0; | ||
|
||
Console.TreatControlCAsInput = true; | ||
|
@@ -114,7 +99,8 @@ public async Task<SecureString> ReadSecureLine(CancellationToken cancellationTok | |
} | ||
else if (previousInputLength > 0 && currentInputLength < previousInputLength) | ||
{ | ||
int row = Console.CursorTop, col = Console.CursorLeft; | ||
int row = await ConsoleProxy.GetCursorTopAsync(cancellationToken); | ||
int col = await ConsoleProxy.GetCursorLeftAsync(cancellationToken); | ||
|
||
// Back up the cursor before clearing the character | ||
col--; | ||
|
@@ -146,10 +132,30 @@ public async Task<SecureString> ReadSecureLine(CancellationToken cancellationTok | |
|
||
private static async Task<ConsoleKeyInfo> ReadKeyAsync(CancellationToken cancellationToken) | ||
{ | ||
return await s_consoleProxy.ReadKeyAsync(cancellationToken); | ||
return await ConsoleProxy.ReadKeyAsync(cancellationToken); | ||
} | ||
|
||
private async Task<string> ReadLine(bool isCommandLine, CancellationToken cancellationToken) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't this be called ReadLineAsync? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't change it because it's an existing method, but it's private so shouldn't be a big deal. |
||
{ | ||
return await this.powerShellContext.InvokeReadLine(isCommandLine, cancellationToken); | ||
} | ||
|
||
/// <summary> | ||
/// Invokes a custom ReadLine method that is similar to but more basic than PSReadLine. | ||
/// This method should be used when PSReadLine is disabled, either by user settings or | ||
/// unsupported PowerShell versions. | ||
/// </summary> | ||
/// <param name="isCommandLine"> | ||
/// Indicates whether ReadLine should act like a command line. | ||
/// </param> | ||
/// <param name="cancellationToken"> | ||
/// The cancellation token that will be checked prior to completing the returned task. | ||
/// </param> | ||
/// <returns> | ||
/// A task object representing the asynchronus operation. The Result property on | ||
/// the task object returns the user input string. | ||
/// </returns> | ||
internal async Task<string> InvokeLegacyReadLine(bool isCommandLine, CancellationToken cancellationToken) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be called |
||
{ | ||
string inputBeforeCompletion = null; | ||
string inputAfterCompletion = null; | ||
|
@@ -160,8 +166,8 @@ private async Task<string> ReadLine(bool isCommandLine, CancellationToken cancel | |
|
||
StringBuilder inputLine = new StringBuilder(); | ||
|
||
int initialCursorCol = Console.CursorLeft; | ||
int initialCursorRow = Console.CursorTop; | ||
int initialCursorCol = await ConsoleProxy.GetCursorLeftAsync(cancellationToken); | ||
int initialCursorRow = await ConsoleProxy.GetCursorTopAsync(cancellationToken); | ||
|
||
int initialWindowLeft = Console.WindowLeft; | ||
int initialWindowTop = Console.WindowTop; | ||
|
@@ -492,8 +498,8 @@ private int CalculateIndexFromCursor( | |
int consoleWidth) | ||
{ | ||
return | ||
((Console.CursorTop - promptStartRow) * consoleWidth) + | ||
Console.CursorLeft - promptStartCol; | ||
((ConsoleProxy.GetCursorTop() - promptStartRow) * consoleWidth) + | ||
ConsoleProxy.GetCursorLeft() - promptStartCol; | ||
} | ||
|
||
private void CalculateCursorFromIndex( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest using string interpolation and use platform independent newlines:
$"Exception occurred while awaiting debug launch task.{Environment.Newline}{Environment.Newline}{e.ToString()}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency I'd like to keep this the same for now. Ultimately we shouldn't be making our own strings while logging at all, that would better be handled by Serilog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine, I just wanted to bring it up as some people are not aware of some of the new C# features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to use
Logger.WriteException
here and it will handle the newlines/indentation.