From 4e4aec0b49d2749e98f9e1724e956255561702e8 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 2 Feb 2024 15:35:38 -0800 Subject: [PATCH 1/3] Handle buffer changes made by OnIdle event handler --- PSReadLine/PublicAPI.cs | 1 + PSReadLine/ReadLine.cs | 26 +++++++++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/PSReadLine/PublicAPI.cs b/PSReadLine/PublicAPI.cs index 3e9696686..e4e311e80 100644 --- a/PSReadLine/PublicAPI.cs +++ b/PSReadLine/PublicAPI.cs @@ -92,6 +92,7 @@ public static void Insert(char c) /// String to insert public static void Insert(string s) { + s = s.Replace("\r\n", "\n"); _singleton.SaveEditItem(EditItemInsertString.Create(s, _singleton._current)); // Use Append if possible because Insert at end makes StringBuilder quite slow. diff --git a/PSReadLine/ReadLine.cs b/PSReadLine/ReadLine.cs index 951349c51..b4b02fbb4 100644 --- a/PSReadLine/ReadLine.cs +++ b/PSReadLine/ReadLine.cs @@ -239,15 +239,31 @@ internal static PSKeyInfo ReadKey() ps.AddScript("[System.Diagnostics.DebuggerHidden()]param() 0", useLocalScope: true); } - // To detect output during possible event processing, see if the cursor moved - // and rerender if so. + // To detect output during possible event processing, see if the cursor moved and rerender if so. var console = _singleton._console; - var y = console.CursorTop; + var buffer = _singleton._buffer; + int y = console.CursorTop, len = buffer.Length; + + // Start the pipeline to process events. ps.Invoke(); - if (y != console.CursorTop) + + // Check if cursor moved, but handle a very special case: an event handler changed our buffer, + // by calling 'Insert' for example. + // + // I know only checking on buffer length change doesn't cover the case where buffer changed but + // the length is the same. However, we mainly want to cover buffer changes made by an 'OnIdle' + // event handler, and we trigger 'OnIdle' event only if the buffer is empty. So, this check is + // efficient and good enough for that main scenario. + // + // When our buffer was changed by an event handler, we assume that was all the event handler did + // and there was no console output. So, we rerender only if there was no buffer change. + if (y != console.CursorTop && buffer.Length == len) { _singleton._initialY = console.CursorTop; - _singleton.Render(); + if (buffer.Length > 0) + { + _singleton.Render(); + } } } } From 7db02f213f0fd06a623ae9858c8ae3d6be8193ee Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 7 Jan 2025 13:27:39 -0800 Subject: [PATCH 2/3] Insert text using the OnIdle event --- PSReadLine/ReadLine.cs | 55 +++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/PSReadLine/ReadLine.cs b/PSReadLine/ReadLine.cs index b4b02fbb4..2079a91ef 100644 --- a/PSReadLine/ReadLine.cs +++ b/PSReadLine/ReadLine.cs @@ -203,24 +203,24 @@ internal static PSKeyInfo ReadKey() // If we timed out, check for event subscribers (which is just // a hint that there might be an event waiting to be processed.) var eventSubscribers = _singleton._engineIntrinsics?.Events.Subscribers; + int bufferLen = _singleton._buffer.Length; if (eventSubscribers?.Count > 0) { bool runPipelineForEventProcessing = false; foreach (var sub in eventSubscribers) { - if (string.Equals(sub.SourceIdentifier, PSEngineEvent.OnIdle, StringComparison.OrdinalIgnoreCase)) + // If the buffer is not empty, let's not consider we are idle because the user is in the middle of typing something. + if (string.Equals(sub.SourceIdentifier, PSEngineEvent.OnIdle, StringComparison.OrdinalIgnoreCase) && bufferLen is 0) { - // If the buffer is not empty, let's not consider we are idle because the user is in the middle of typing something. - if (_singleton._buffer.Length > 0) - { - continue; - } - - // There is an OnIdle event subscriber and we are idle because we timed out and the buffer is empty. - // Normally PowerShell generates this event, but PowerShell assumes the engine is not idle because - // it called PSConsoleHostReadLine which isn't returning. So we generate the event instead. + // There is an 'OnIdle' event subscriber and we are idle because we timed out and the buffer is empty. + // Normally PowerShell generates this event, but now PowerShell assumes the engine is not idle because + // it called 'PSConsoleHostReadLine' which isn't returning. So we generate the event instead. runPipelineForEventProcessing = true; - _singleton._engineIntrinsics.Events.GenerateEvent(PSEngineEvent.OnIdle, null, null, null); + _singleton._engineIntrinsics.Events.GenerateEvent( + PSEngineEvent.OnIdle, + sender: null, + args: null, + extraData: null); // Break out so we don't genreate more than one 'OnIdle' event for a timeout. break; @@ -240,27 +240,32 @@ internal static PSKeyInfo ReadKey() } // To detect output during possible event processing, see if the cursor moved and rerender if so. - var console = _singleton._console; - var buffer = _singleton._buffer; - int y = console.CursorTop, len = buffer.Length; + int cursorTop = _singleton._console.CursorTop; // Start the pipeline to process events. ps.Invoke(); - // Check if cursor moved, but handle a very special case: an event handler changed our buffer, - // by calling 'Insert' for example. + // Check if any event handler writes console output to the best of our effort, and adjust the initial coordinates in that case. // - // I know only checking on buffer length change doesn't cover the case where buffer changed but - // the length is the same. However, we mainly want to cover buffer changes made by an 'OnIdle' - // event handler, and we trigger 'OnIdle' event only if the buffer is empty. So, this check is - // efficient and good enough for that main scenario. + // I say "to the best of our effort" because the delegate handler for an event will mostly run on a background thread, and thus + // there is no guarantee about when the delegate would finish. So in an extreme case, there could be race conditions in console + // read/write: we are reading 'CursorTop' while the delegate is writing console output on a different thread. + // There is no much we can do about that extreme case. However, our focus here is the 'OnIdle' event, and its handler is usually + // a script block, which will run within the 'ps.Invoke()' call above. // - // When our buffer was changed by an event handler, we assume that was all the event handler did - // and there was no console output. So, we rerender only if there was no buffer change. - if (y != console.CursorTop && buffer.Length == len) + // We detect new console output by checking if cursor top changed, but handle a very special case: an event handler changed our + // buffer, by calling 'Insert' for example. + // I know only checking on buffer length change doesn't cover the case where buffer changed but the length is the same. However, + // we mainly want to cover buffer changes made by an 'OnIdle' event handler, and we trigger 'OnIdle' event only if the buffer is + // empty. So, this check is efficient and good enough for that main scenario. + // When our buffer was changed by an event handler, we assume that was all the event handler did and there was no direct console + // output. So, we adjust the initial coordinates only if cursor top changed but there was no buffer change. + int newCursorTop = _singleton._console.CursorTop; + int newBufferLen = _singleton._buffer.Length; + if (cursorTop != newCursorTop && bufferLen == newBufferLen) { - _singleton._initialY = console.CursorTop; - if (buffer.Length > 0) + _singleton._initialY = newCursorTop; + if (bufferLen > 0) { _singleton.Render(); } From 6e1297d3e1ffb9a1ebb3fc88c9a4bb84e01a6ef9 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 3 Feb 2025 15:17:49 -0800 Subject: [PATCH 3/3] minor update --- PSReadLine/ReadLine.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/PSReadLine/ReadLine.cs b/PSReadLine/ReadLine.cs index 2079a91ef..02a0455b0 100644 --- a/PSReadLine/ReadLine.cs +++ b/PSReadLine/ReadLine.cs @@ -209,9 +209,14 @@ internal static PSKeyInfo ReadKey() bool runPipelineForEventProcessing = false; foreach (var sub in eventSubscribers) { - // If the buffer is not empty, let's not consider we are idle because the user is in the middle of typing something. - if (string.Equals(sub.SourceIdentifier, PSEngineEvent.OnIdle, StringComparison.OrdinalIgnoreCase) && bufferLen is 0) + if (string.Equals(sub.SourceIdentifier, PSEngineEvent.OnIdle, StringComparison.OrdinalIgnoreCase)) { + // If the buffer is not empty, let's not consider we are idle because the user is in the middle of typing something. + if (bufferLen > 0) + { + continue; + } + // There is an 'OnIdle' event subscriber and we are idle because we timed out and the buffer is empty. // Normally PowerShell generates this event, but now PowerShell assumes the engine is not idle because // it called 'PSConsoleHostReadLine' which isn't returning. So we generate the event instead.