From 2d635071e1f7318f937c3bc36d8f8678b3b6e483 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Fri, 10 Jul 2020 13:01:56 +0200 Subject: [PATCH 1/6] Console.Unix: make Console.OpenStandardInput Stream aware of terminal When performing OpenStandardInput against a terminal, perform Reads on a line-by-line basis and perform appropriate processing and echoing. --- .../src/System/ConsolePal.Unix.cs | 22 +- .../src/System/IO/StdInReader.cs | 228 +++++++++++------- .../src/System/IO/SyncTextReader.Unix.cs | 3 + 3 files changed, 162 insertions(+), 91 deletions(-) diff --git a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs index 098601bfdbe92e..c9cc26dd87e28a 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs @@ -43,7 +43,8 @@ internal static class ConsolePal public static Stream OpenStandardInput() { - return new UnixConsoleStream(SafeFileHandleHelper.Open(() => Interop.Sys.Dup(Interop.Sys.FileDescriptors.STDIN_FILENO)), FileAccess.Read); + return new UnixConsoleStream(SafeFileHandleHelper.Open(() => Interop.Sys.Dup(Interop.Sys.FileDescriptors.STDIN_FILENO)), FileAccess.Read, + useReadLine: !Console.IsInputRedirected); } public static Stream OpenStandardOutput() @@ -68,7 +69,7 @@ public static Encoding OutputEncoding private static SyncTextReader? s_stdInReader; - private static SyncTextReader StdInReader + internal static SyncTextReader StdInReader { get { @@ -1410,15 +1411,19 @@ private sealed class UnixConsoleStream : ConsoleStream /// The file descriptor for the opened file. private readonly SafeFileHandle _handle; + private readonly bool _useReadLine; + /// Initialize the stream. /// The file handle wrapped by this stream. /// FileAccess.Read or FileAccess.Write. - internal UnixConsoleStream(SafeFileHandle handle, FileAccess access) + /// Use ReadLine API for reading. + internal UnixConsoleStream(SafeFileHandle handle, FileAccess access, bool useReadLine = false) : base(access) { Debug.Assert(handle != null, "Expected non-null console handle"); Debug.Assert(!handle.IsInvalid, "Expected valid console handle"); _handle = handle; + _useReadLine = useReadLine; } protected override void Dispose(bool disposing) @@ -1430,11 +1435,18 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } - public override int Read(byte[] buffer, int offset, int count) + public unsafe override int Read(byte[] buffer, int offset, int count) { ValidateRead(buffer, offset, count); - return ConsolePal.Read(_handle, buffer, offset, count); + if (_useReadLine) + { + return ConsolePal.StdInReader.ReadLine(buffer, offset, count); + } + else + { + return ConsolePal.Read(_handle, buffer, offset, count); + } } public override void Write(byte[] buffer, int offset, int count) diff --git a/src/libraries/System.Console/src/System/IO/StdInReader.cs b/src/libraries/System.Console/src/System/IO/StdInReader.cs index 99ebbd34c61496..471cd9577ef3ef 100644 --- a/src/libraries/System.Console/src/System/IO/StdInReader.cs +++ b/src/libraries/System.Console/src/System/IO/StdInReader.cs @@ -22,6 +22,7 @@ internal sealed class StdInReader : TextReader private readonly Stack _tmpKeys = new Stack(); // temporary working stack; should be empty outside of ReadLine private readonly Stack _availableKeys = new Stack(); // a queue of already processed key infos available for reading private readonly Encoding _encoding; + private Encoder? _bufferedReadEncoder; private char[] _unprocessedBufferToBeRead; // Buffer that might have already been read from stdin but not yet processed. private const int BytesToBeRead = 1024; // No. of bytes to be read from the stream at a time. @@ -79,129 +80,184 @@ internal unsafe int ReadStdin(byte* buffer, int bufferSize) public override string? ReadLine() { - return ReadLine(consumeKeys: true); + return ReadLine(consumeKeys: true).line; } - private string? ReadLine(bool consumeKeys) + public int ReadLine(byte[] buffer, int offset, int count) { + return ReadLine(consumeKeys: true, buffer, offset, count).bytesRead; + } + + private (string? line, int bytesRead) ReadLine(bool consumeKeys, byte[]? buffer = null, int offset = 0, int count = 0) + { + // This method blocks until a line was read or EOF was reached. + // When buffer is null, and consumeKeys is true, the line is returned as a string. + // When buffer is not null, the line is encoded into the buffer. + // If the buffer is too small for the line, successive calls will return the remainder. Debug.Assert(_tmpKeys.Count == 0); - string? readLineStr = null; - Interop.Sys.InitializeConsoleBeforeRead(); - try + // Don't carry over chars from buffer read followed by non-buffer read. + if (buffer == null) { - // Read key-by-key until we've read a line. - while (true) + _readLineSB.Clear(); + } + // read into emty buffer. + else if (count == 0) + { + return (null, 0); + } + + // For a buffer read, first use remaining chars from _readLineSB. + if (buffer == null || _readLineSB.Length == 0) + { + Interop.Sys.InitializeConsoleBeforeRead(); + try { - // Read the next key. This may come from previously read keys, from previously read but - // unprocessed data, or from an actual stdin read. - bool previouslyProcessed; - ConsoleKeyInfo keyInfo = ReadKey(out previouslyProcessed); - if (!consumeKeys && keyInfo.Key != ConsoleKey.Backspace) // backspace is the only character not written out in the below if/elses. + // Read key-by-key until we've read a line. + while (true) { - _tmpKeys.Push(keyInfo); - } + // Read the next key. This may come from previously read keys, from previously read but + // unprocessed data, or from an actual stdin read. + bool previouslyProcessed; + ConsoleKeyInfo keyInfo = ReadKey(out previouslyProcessed); + if (!consumeKeys && keyInfo.Key != ConsoleKey.Backspace) // backspace is the only character not written out in the below if/elses. + { + _tmpKeys.Push(keyInfo); + } - // Handle the next key. Since for other functions we may have ended up reading some of the user's - // input, we need to be able to handle manually processing that input, and so we do that processing - // for all input. As such, we need to special-case a few characters, e.g. recognizing when Enter is - // pressed to end a line. We also need to handle Backspace specially, to fix up both our buffer of - // characters and the position of the cursor. More advanced processing would be possible, but we - // try to keep this very simple, at least for now. - if (keyInfo.Key == ConsoleKey.Enter) - { - readLineStr = _readLineSB.ToString(); - _readLineSB.Clear(); - if (!previouslyProcessed) + // Handle the next key. Since for other functions we may have ended up reading some of the user's + // input, we need to be able to handle manually processing that input, and so we do that processing + // for all input. As such, we need to special-case a few characters, e.g. recognizing when Enter is + // pressed to end a line. We also need to handle Backspace specially, to fix up both our buffer of + // characters and the position of the cursor. More advanced processing would be possible, but we + // try to keep this very simple, at least for now. + if (keyInfo.Key == ConsoleKey.Enter) { - Console.WriteLine(); + if (buffer != null) + { + _readLineSB.Append('\n'); + } + if (!previouslyProcessed) + { + Console.WriteLine(); + } + break; } - break; - } - else if (IsEol(keyInfo.KeyChar)) - { - string line = _readLineSB.ToString(); - _readLineSB.Clear(); - if (line.Length > 0) + else if (IsEol(keyInfo.KeyChar)) { - readLineStr = line; + if (_readLineSB.Length == 0) + { + return (null, 0); + } + break; } - break; - } - else if (keyInfo.Key == ConsoleKey.Backspace) - { - int len = _readLineSB.Length; - if (len > 0) + else if (keyInfo.Key == ConsoleKey.Backspace) { - _readLineSB.Length = len - 1; - if (!previouslyProcessed) + int len = _readLineSB.Length; + if (len > 0) { - // The ReadLine input may wrap accross terminal rows and we need to handle that. - // note: ConsolePal will cache the cursor position to avoid making many slow cursor position fetch operations. - if (ConsolePal.TryGetCursorPosition(out int left, out int top, reinitializeForRead: true) && - left == 0 && top > 0) + _readLineSB.Length = len - 1; + if (!previouslyProcessed) { - if (s_clearToEol == null) + // The ReadLine input may wrap accross terminal rows and we need to handle that. + // note: ConsolePal will cache the cursor position to avoid making many slow cursor position fetch operations. + if (ConsolePal.TryGetCursorPosition(out int left, out int top, reinitializeForRead: true) && + left == 0 && top > 0) { - s_clearToEol = ConsolePal.TerminalFormatStrings.Instance.ClrEol ?? string.Empty; + if (s_clearToEol == null) + { + s_clearToEol = ConsolePal.TerminalFormatStrings.Instance.ClrEol ?? string.Empty; + } + + // Move to end of previous line + ConsolePal.SetCursorPosition(ConsolePal.WindowWidth - 1, top - 1); + // Clear from cursor to end of the line + ConsolePal.WriteStdoutAnsiString(s_clearToEol, mayChangeCursorPosition: false); } - - // Move to end of previous line - ConsolePal.SetCursorPosition(ConsolePal.WindowWidth - 1, top - 1); - // Clear from cursor to end of the line - ConsolePal.WriteStdoutAnsiString(s_clearToEol, mayChangeCursorPosition: false); - } - else - { - if (s_moveLeftString == null) + else { - string? moveLeft = ConsolePal.TerminalFormatStrings.Instance.CursorLeft; - s_moveLeftString = !string.IsNullOrEmpty(moveLeft) ? moveLeft + " " + moveLeft : string.Empty; - } + if (s_moveLeftString == null) + { + string? moveLeft = ConsolePal.TerminalFormatStrings.Instance.CursorLeft; + s_moveLeftString = !string.IsNullOrEmpty(moveLeft) ? moveLeft + " " + moveLeft : string.Empty; + } - Console.Write(s_moveLeftString); + Console.Write(s_moveLeftString); + } } } } - } - else if (keyInfo.Key == ConsoleKey.Tab) - { - _readLineSB.Append(keyInfo.KeyChar); - if (!previouslyProcessed) + else if (keyInfo.Key == ConsoleKey.Tab) { - Console.Write(' '); + _readLineSB.Append(keyInfo.KeyChar); + if (!previouslyProcessed) + { + Console.Write(' '); + } } - } - else if (keyInfo.Key == ConsoleKey.Clear) - { - _readLineSB.Clear(); - if (!previouslyProcessed) + else if (keyInfo.Key == ConsoleKey.Clear) { - Console.Clear(); + _readLineSB.Clear(); + if (!previouslyProcessed) + { + Console.Clear(); + } } - } - else if (keyInfo.KeyChar != '\0') - { - _readLineSB.Append(keyInfo.KeyChar); - if (!previouslyProcessed) + else if (keyInfo.KeyChar != '\0') { - Console.Write(keyInfo.KeyChar); + _readLineSB.Append(keyInfo.KeyChar); + if (!previouslyProcessed) + { + Console.Write(keyInfo.KeyChar); + } } } } + finally + { + Interop.Sys.UninitializeConsoleAfterRead(); + + // If we're not consuming the read input, make the keys available for a future read + while (_tmpKeys.Count > 0) + { + _availableKeys.Push(_tmpKeys.Pop()); + } + } } - finally - { - Interop.Sys.UninitializeConsoleAfterRead(); - // If we're not consuming the read input, make the keys available for a future read - while (_tmpKeys.Count > 0) + string? line = null; + int bytesUsedTotal = 0; + if (buffer == null) + { + if (consumeKeys) { - _availableKeys.Push(_tmpKeys.Pop()); + line = _readLineSB.ToString(); } + _readLineSB.Clear(); } + else + { + // Encode _readLineSB in buffer. + Encoder encoder = _bufferedReadEncoder ??= _encoding.GetEncoder(); - return readLineStr; + int charsUsedTotal = 0; + Span destination = buffer.AsSpan(offset, count); + foreach (ReadOnlyMemory chunk in _readLineSB.GetChunks()) + { + encoder.Convert(chunk.Span, destination, flush: false, out int charsUsed, out int bytesUsed, out bool completed); + destination = destination.Slice(bytesUsed); + bytesUsedTotal += bytesUsed; + charsUsedTotal += charsUsed; + + if (charsUsed == 0) + { + break; + } + } + + _readLineSB.Remove(0, charsUsedTotal); + } + return (line, bytesUsedTotal); } public override int Read() => ReadOrPeek(peek: false); diff --git a/src/libraries/System.Console/src/System/IO/SyncTextReader.Unix.cs b/src/libraries/System.Console/src/System/IO/SyncTextReader.Unix.cs index f98499db4387a7..9e68d74c1b0023 100644 --- a/src/libraries/System.Console/src/System/IO/SyncTextReader.Unix.cs +++ b/src/libraries/System.Console/src/System/IO/SyncTextReader.Unix.cs @@ -39,5 +39,8 @@ public bool KeyAvailable } } } + + public int ReadLine(byte[] buffer, int offset, int count) + => Inner.ReadLine(buffer, offset, count); } } From cb82cc9dc7bc5b1aeea25504519b288694510659 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 14 Jul 2020 22:54:20 +0200 Subject: [PATCH 2/6] PR feedback --- .../src/System/ConsolePal.Unix.cs | 2 +- .../src/System/IO/StdInReader.cs | 267 +++++++++--------- 2 files changed, 131 insertions(+), 138 deletions(-) diff --git a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs index c9cc26dd87e28a..017c55e8c8a3ec 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs @@ -1435,7 +1435,7 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } - public unsafe override int Read(byte[] buffer, int offset, int count) + public override int Read(byte[] buffer, int offset, int count) { ValidateRead(buffer, offset, count); diff --git a/src/libraries/System.Console/src/System/IO/StdInReader.cs b/src/libraries/System.Console/src/System/IO/StdInReader.cs index 471cd9577ef3ef..bad11c057b0f05 100644 --- a/src/libraries/System.Console/src/System/IO/StdInReader.cs +++ b/src/libraries/System.Console/src/System/IO/StdInReader.cs @@ -22,7 +22,7 @@ internal sealed class StdInReader : TextReader private readonly Stack _tmpKeys = new Stack(); // temporary working stack; should be empty outside of ReadLine private readonly Stack _availableKeys = new Stack(); // a queue of already processed key infos available for reading private readonly Encoding _encoding; - private Encoder? _bufferedReadEncoder; + private Encoder? _bufferReadEncoder; private char[] _unprocessedBufferToBeRead; // Buffer that might have already been read from stdin but not yet processed. private const int BytesToBeRead = 1024; // No. of bytes to be read from the stream at a time. @@ -80,184 +80,177 @@ internal unsafe int ReadStdin(byte* buffer, int bufferSize) public override string? ReadLine() { - return ReadLine(consumeKeys: true).line; - } + // Don't carry over chars from ReadLine buffer method. + _readLineSB.Clear(); - public int ReadLine(byte[] buffer, int offset, int count) - { - return ReadLine(consumeKeys: true, buffer, offset, count).bytesRead; + bool isEnter = ReadLineCore(consumeKeys: true); + string? line = null; + if (isEnter || _readLineSB.Length > 0) + { + line = _readLineSB.ToString(); + } + else + { + _readLineSB.Clear(); + } + return line; } - private (string? line, int bytesRead) ReadLine(bool consumeKeys, byte[]? buffer = null, int offset = 0, int count = 0) + public int ReadLine(byte[] buffer, int offset, int count) { - // This method blocks until a line was read or EOF was reached. - // When buffer is null, and consumeKeys is true, the line is returned as a string. - // When buffer is not null, the line is encoded into the buffer. - // If the buffer is too small for the line, successive calls will return the remainder. - Debug.Assert(_tmpKeys.Count == 0); + if (count == 0) + { + return 0; + } - // Don't carry over chars from buffer read followed by non-buffer read. - if (buffer == null) + // Don't read a new line if there are remaining characters in the StringBuilder. + if (_readLineSB.Length == 0) { - _readLineSB.Clear(); + bool isEnter = ReadLineCore(consumeKeys: true); + if (isEnter) + { + _readLineSB.Append('\n'); + } } - // read into emty buffer. - else if (count == 0) + + // Encode line into buffer. + Encoder encoder = _bufferReadEncoder ??= _encoding.GetEncoder(); + int bytesUsedTotal = 0; + int charsUsedTotal = 0; + Span destination = buffer.AsSpan(offset, count); + foreach (ReadOnlyMemory chunk in _readLineSB.GetChunks()) { - return (null, 0); + encoder.Convert(chunk.Span, destination, flush: false, out int charsUsed, out int bytesUsed, out bool completed); + destination = destination.Slice(bytesUsed); + bytesUsedTotal += bytesUsed; + charsUsedTotal += charsUsed; + + if (charsUsed == 0) + { + break; + } } + _readLineSB.Remove(0, charsUsedTotal); + return bytesUsedTotal; + } - // For a buffer read, first use remaining chars from _readLineSB. - if (buffer == null || _readLineSB.Length == 0) + // Reads a line in _readLineSB when consumeKeys is true, + // or _availableKeys when consumeKeys is false. + // Returns whether the line was terminated using the Enter key. + private bool ReadLineCore(bool consumeKeys) + { + Debug.Assert(_tmpKeys.Count == 0); + Interop.Sys.InitializeConsoleBeforeRead(); + try { - Interop.Sys.InitializeConsoleBeforeRead(); - try + // Read key-by-key until we've read a line. + while (true) { - // Read key-by-key until we've read a line. - while (true) + // Read the next key. This may come from previously read keys, from previously read but + // unprocessed data, or from an actual stdin read. + bool previouslyProcessed; + ConsoleKeyInfo keyInfo = ReadKey(out previouslyProcessed); + if (!consumeKeys && keyInfo.Key != ConsoleKey.Backspace) // backspace is the only character not written out in the below if/elses. { - // Read the next key. This may come from previously read keys, from previously read but - // unprocessed data, or from an actual stdin read. - bool previouslyProcessed; - ConsoleKeyInfo keyInfo = ReadKey(out previouslyProcessed); - if (!consumeKeys && keyInfo.Key != ConsoleKey.Backspace) // backspace is the only character not written out in the below if/elses. - { - _tmpKeys.Push(keyInfo); - } + _tmpKeys.Push(keyInfo); + } - // Handle the next key. Since for other functions we may have ended up reading some of the user's - // input, we need to be able to handle manually processing that input, and so we do that processing - // for all input. As such, we need to special-case a few characters, e.g. recognizing when Enter is - // pressed to end a line. We also need to handle Backspace specially, to fix up both our buffer of - // characters and the position of the cursor. More advanced processing would be possible, but we - // try to keep this very simple, at least for now. - if (keyInfo.Key == ConsoleKey.Enter) - { - if (buffer != null) - { - _readLineSB.Append('\n'); - } - if (!previouslyProcessed) - { - Console.WriteLine(); - } - break; - } - else if (IsEol(keyInfo.KeyChar)) + // Handle the next key. Since for other functions we may have ended up reading some of the user's + // input, we need to be able to handle manually processing that input, and so we do that processing + // for all input. As such, we need to special-case a few characters, e.g. recognizing when Enter is + // pressed to end a line. We also need to handle Backspace specially, to fix up both our buffer of + // characters and the position of the cursor. More advanced processing would be possible, but we + // try to keep this very simple, at least for now. + if (keyInfo.Key == ConsoleKey.Enter) + { + if (!previouslyProcessed) { - if (_readLineSB.Length == 0) - { - return (null, 0); - } - break; + Console.WriteLine(); } - else if (keyInfo.Key == ConsoleKey.Backspace) + return true; + } + else if (IsEol(keyInfo.KeyChar)) + { + return false; + } + else if (keyInfo.Key == ConsoleKey.Backspace) + { + int len = _readLineSB.Length; + if (len > 0) { - int len = _readLineSB.Length; - if (len > 0) + _readLineSB.Length = len - 1; + if (!previouslyProcessed) { - _readLineSB.Length = len - 1; - if (!previouslyProcessed) + // The ReadLine input may wrap accross terminal rows and we need to handle that. + // note: ConsolePal will cache the cursor position to avoid making many slow cursor position fetch operations. + if (ConsolePal.TryGetCursorPosition(out int left, out int top, reinitializeForRead: true) && + left == 0 && top > 0) { - // The ReadLine input may wrap accross terminal rows and we need to handle that. - // note: ConsolePal will cache the cursor position to avoid making many slow cursor position fetch operations. - if (ConsolePal.TryGetCursorPosition(out int left, out int top, reinitializeForRead: true) && - left == 0 && top > 0) + if (s_clearToEol == null) { - if (s_clearToEol == null) - { - s_clearToEol = ConsolePal.TerminalFormatStrings.Instance.ClrEol ?? string.Empty; - } - - // Move to end of previous line - ConsolePal.SetCursorPosition(ConsolePal.WindowWidth - 1, top - 1); - // Clear from cursor to end of the line - ConsolePal.WriteStdoutAnsiString(s_clearToEol, mayChangeCursorPosition: false); + s_clearToEol = ConsolePal.TerminalFormatStrings.Instance.ClrEol ?? string.Empty; } - else - { - if (s_moveLeftString == null) - { - string? moveLeft = ConsolePal.TerminalFormatStrings.Instance.CursorLeft; - s_moveLeftString = !string.IsNullOrEmpty(moveLeft) ? moveLeft + " " + moveLeft : string.Empty; - } - Console.Write(s_moveLeftString); + // Move to end of previous line + ConsolePal.SetCursorPosition(ConsolePal.WindowWidth - 1, top - 1); + // Clear from cursor to end of the line + ConsolePal.WriteStdoutAnsiString(s_clearToEol, mayChangeCursorPosition: false); + } + else + { + if (s_moveLeftString == null) + { + string? moveLeft = ConsolePal.TerminalFormatStrings.Instance.CursorLeft; + s_moveLeftString = !string.IsNullOrEmpty(moveLeft) ? moveLeft + " " + moveLeft : string.Empty; } + + Console.Write(s_moveLeftString); } } } - else if (keyInfo.Key == ConsoleKey.Tab) + } + else if (keyInfo.Key == ConsoleKey.Tab) + { + if (consumeKeys) { _readLineSB.Append(keyInfo.KeyChar); - if (!previouslyProcessed) - { - Console.Write(' '); - } } - else if (keyInfo.Key == ConsoleKey.Clear) + if (!previouslyProcessed) { - _readLineSB.Clear(); - if (!previouslyProcessed) - { - Console.Clear(); - } + Console.Write(' '); } - else if (keyInfo.KeyChar != '\0') + } + else if (keyInfo.Key == ConsoleKey.Clear) + { + _readLineSB.Clear(); + if (!previouslyProcessed) { - _readLineSB.Append(keyInfo.KeyChar); - if (!previouslyProcessed) - { - Console.Write(keyInfo.KeyChar); - } + Console.Clear(); } } - } - finally - { - Interop.Sys.UninitializeConsoleAfterRead(); - - // If we're not consuming the read input, make the keys available for a future read - while (_tmpKeys.Count > 0) + else if (keyInfo.KeyChar != '\0') { - _availableKeys.Push(_tmpKeys.Pop()); + if (consumeKeys) + { + _readLineSB.Append(keyInfo.KeyChar); + } + if (!previouslyProcessed) + { + Console.Write(keyInfo.KeyChar); + } } } } - - string? line = null; - int bytesUsedTotal = 0; - if (buffer == null) - { - if (consumeKeys) - { - line = _readLineSB.ToString(); - } - _readLineSB.Clear(); - } - else + finally { - // Encode _readLineSB in buffer. - Encoder encoder = _bufferedReadEncoder ??= _encoding.GetEncoder(); + Interop.Sys.UninitializeConsoleAfterRead(); - int charsUsedTotal = 0; - Span destination = buffer.AsSpan(offset, count); - foreach (ReadOnlyMemory chunk in _readLineSB.GetChunks()) + // If we're not consuming the read input, make the keys available for a future read + while (_tmpKeys.Count > 0) { - encoder.Convert(chunk.Span, destination, flush: false, out int charsUsed, out int bytesUsed, out bool completed); - destination = destination.Slice(bytesUsed); - bytesUsedTotal += bytesUsed; - charsUsedTotal += charsUsed; - - if (charsUsed == 0) - { - break; - } + _availableKeys.Push(_tmpKeys.Pop()); } - - _readLineSB.Remove(0, charsUsedTotal); } - return (line, bytesUsedTotal); } public override int Read() => ReadOrPeek(peek: false); @@ -269,7 +262,7 @@ private int ReadOrPeek(bool peek) // If there aren't any keys in our processed keys stack, read a line to populate it. if (_availableKeys.Count == 0) { - ReadLine(consumeKeys: false); + ReadLineCore(consumeKeys: false); } // Now if there are keys, use the first. From d922bed28998c13a29e52196d6510c9551bb2cf1 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 14 Jul 2020 23:04:29 +0200 Subject: [PATCH 3/6] Move StringBuffer.Clear into ReadLineCore --- src/libraries/System.Console/src/System/IO/StdInReader.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Console/src/System/IO/StdInReader.cs b/src/libraries/System.Console/src/System/IO/StdInReader.cs index bad11c057b0f05..43aad111ebfb52 100644 --- a/src/libraries/System.Console/src/System/IO/StdInReader.cs +++ b/src/libraries/System.Console/src/System/IO/StdInReader.cs @@ -80,9 +80,6 @@ internal unsafe int ReadStdin(byte* buffer, int bufferSize) public override string? ReadLine() { - // Don't carry over chars from ReadLine buffer method. - _readLineSB.Clear(); - bool isEnter = ReadLineCore(consumeKeys: true); string? line = null; if (isEnter || _readLineSB.Length > 0) @@ -140,6 +137,10 @@ public int ReadLine(byte[] buffer, int offset, int count) private bool ReadLineCore(bool consumeKeys) { Debug.Assert(_tmpKeys.Count == 0); + + // Don't carry over chars from previous ReadLine call. + _readLineSB.Clear(); + Interop.Sys.InitializeConsoleBeforeRead(); try { From d3e87db9bc0c7fe511c07135e8f22b9b8dc5e7ae Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 15 Jul 2020 10:19:10 +0200 Subject: [PATCH 4/6] Fix StringBuffer clear on ReadLine --- src/libraries/System.Console/src/System/IO/StdInReader.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libraries/System.Console/src/System/IO/StdInReader.cs b/src/libraries/System.Console/src/System/IO/StdInReader.cs index 43aad111ebfb52..6396176fbc905a 100644 --- a/src/libraries/System.Console/src/System/IO/StdInReader.cs +++ b/src/libraries/System.Console/src/System/IO/StdInReader.cs @@ -85,9 +85,6 @@ internal unsafe int ReadStdin(byte* buffer, int bufferSize) if (isEnter || _readLineSB.Length > 0) { line = _readLineSB.ToString(); - } - else - { _readLineSB.Clear(); } return line; From b723491110f900734fa78324ad25151b2620ac37 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 15 Jul 2020 10:19:22 +0200 Subject: [PATCH 5/6] Add test --- .../tests/ManualTests/ManualTests.cs | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Console/tests/ManualTests/ManualTests.cs b/src/libraries/System.Console/tests/ManualTests/ManualTests.cs index 9bbec557dbb732..4d60306a153502 100644 --- a/src/libraries/System.Console/tests/ManualTests/ManualTests.cs +++ b/src/libraries/System.Console/tests/ManualTests/ManualTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Threading.Tasks; +using System.IO; using Xunit; namespace System @@ -23,6 +24,26 @@ public static void ReadLine(bool consoleIn) AssertUserExpectedResults("the characters you typed properly echoed as you typed"); } + [ConditionalFact(nameof(ManualTestsEnabled))] + public static void ReadLineFromOpenStandardInput() + { + string expectedLine = "aab"; + + // Use Console.ReadLine + Console.WriteLine($"Please type 'a' 3 times, press 'Backspace' to erase 1, then type a single 'b' and press 'Enter'."); + string result = Console.ReadLine(); + Assert.Equal(expectedLine, result); + AssertUserExpectedResults("the characters you typed properly echoed as you typed"); + + // ReadLine from Console.OpenStandardInput + Console.WriteLine($"Please type 'a' 3 times, press 'Backspace' to erase 1, then type a single 'b' and press 'Enter'."); + using Stream inputStream = Console.OpenStandardInput(); + using StreamReader reader = new StreamReader(inputStream); + result = reader.ReadLine(); + Assert.Equal(expectedLine, result); + AssertUserExpectedResults("the characters you typed properly echoed as you typed"); + } + [ConditionalFact(nameof(ManualTestsEnabled))] public static void ReadLine_BackSpaceCanMoveAccrossWrappedLines() { @@ -117,18 +138,6 @@ static object[] MkConsoleKeyInfo (char keyChar, ConsoleKey consoleKey, ConsoleMo } } - [ConditionalFact(nameof(ManualTestsEnabled))] - public static void OpenStandardInput() - { - Console.WriteLine("Please type \"console\" (without the quotes). You shouldn't see it as you type:"); - var stream = Console.OpenStandardInput(); - var textReader = new System.IO.StreamReader(stream); - var result = textReader.ReadLine(); - - Assert.Equal("console", result); - AssertUserExpectedResults("\"console\" correctly not echoed as you typed it"); - } - [ConditionalFact(nameof(ManualTestsEnabled))] public static void ConsoleOutWriteLine() { From 74c2cb22d551dff333a3120b3c68a0518b1a3a3b Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 12 Aug 2020 21:45:51 +0100 Subject: [PATCH 6/6] fix manual tests for Windows --- .../tests/ManualTests/ManualTests.cs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Console/tests/ManualTests/ManualTests.cs b/src/libraries/System.Console/tests/ManualTests/ManualTests.cs index 4d60306a153502..5928b7a34132b8 100644 --- a/src/libraries/System.Console/tests/ManualTests/ManualTests.cs +++ b/src/libraries/System.Console/tests/ManualTests/ManualTests.cs @@ -57,6 +57,7 @@ public static void ReadLine_BackSpaceCanMoveAccrossWrappedLines() } [ConditionalFact(nameof(ManualTestsEnabled))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/40735", TestPlatforms.Windows)] public static void InPeek() { Console.WriteLine("Please type \"peek\" (without the quotes). You should see it as you type:"); @@ -112,19 +113,11 @@ static string RenderKeyChord(ConsoleKeyInfo key) public static IEnumerable GetKeyChords() { - yield return MkConsoleKeyInfo('\x01', ConsoleKey.A, ConsoleModifiers.Control); - yield return MkConsoleKeyInfo('\x01', ConsoleKey.A, ConsoleModifiers.Control | ConsoleModifiers.Alt); + yield return MkConsoleKeyInfo('\x02', ConsoleKey.B, ConsoleModifiers.Control); + yield return MkConsoleKeyInfo(OperatingSystem.IsWindows() ? '\x00' : '\x02', ConsoleKey.B, ConsoleModifiers.Control | ConsoleModifiers.Alt); yield return MkConsoleKeyInfo('\r', ConsoleKey.Enter, (ConsoleModifiers)0); - - if (OperatingSystem.IsWindows()) - { - // windows will report '\n' as 'Ctrl+Enter', which is typically not picked up by Unix terminals - yield return MkConsoleKeyInfo('\n', ConsoleKey.Enter, ConsoleModifiers.Control); - } - else - { - yield return MkConsoleKeyInfo('\n', ConsoleKey.J, ConsoleModifiers.Control); - } + // windows will report '\n' as 'Ctrl+Enter', which is typically not picked up by Unix terminals + yield return MkConsoleKeyInfo('\n', OperatingSystem.IsWindows() ? ConsoleKey.Enter : ConsoleKey.J, ConsoleModifiers.Control); static object[] MkConsoleKeyInfo (char keyChar, ConsoleKey consoleKey, ConsoleModifiers modifiers) { @@ -225,7 +218,7 @@ public static void CursorPositionAndArrowKeys() } } - AssertUserExpectedResults("the arrow keys move around the screen as expected with no other bad artificts"); + AssertUserExpectedResults("the arrow keys move around the screen as expected with no other bad artifacts"); } [ConditionalFact(nameof(ManualTestsEnabled))]