From c2baffc69664dbfa3ec42a1afe9b2196c2e6a98d Mon Sep 17 00:00:00 2001 From: Will Fuqua Date: Sun, 28 Jun 2026 23:01:19 +0700 Subject: [PATCH] Fix history next/previous when on long lines that wrap Fixes https://github.com/waf/CSharpRepl/issues/247 --- src/PrettyPrompt/History/HistoryLog.cs | 41 +++++++++-- tests/PrettyPrompt.Tests/HistoryTests.cs | 86 ++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 4 deletions(-) diff --git a/src/PrettyPrompt/History/HistoryLog.cs b/src/PrettyPrompt/History/HistoryLog.cs index 3421c74..2ab737d 100644 --- a/src/PrettyPrompt/History/HistoryLog.cs +++ b/src/PrettyPrompt/History/HistoryLog.cs @@ -41,6 +41,12 @@ internal sealed class HistoryLog : IKeyPressHandler /// private readonly Stack historyPath = new(); + /// + /// The document contents as of the last time we navigated history. When the current contents differ from this, it means the user + /// has edited the buffer, so we detach from . + /// + private string contentsAfterLastNavigation = string.Empty; + private bool historyEntryWasUsedLastTime; /// @@ -96,14 +102,29 @@ public async Task OnKeyUp(KeyPress key, CancellationToken cancellationToken) if (history.Count == 0 || key.Handled) return; var contents = codePane.Document.GetText(); - if (contents.Contains('\n') && !allowInMultilineStatement) + + // Detach from the active history path once the user edits the buffer (its text no longer matches the entry we navigated to). + if (contents != contentsAfterLastNavigation) + { + historyPath.Clear(); + } + + if (contents.Contains('\n') && + !allowInMultilineStatement && + !keyBindings.HistoryNext.Matches(key.ConsoleKeyInfo)) { - //we do not want to cycle in history in multiline documents + // we do not want to cycle in history in multiline documents, unless we're moving down so the user can get back to their original prompt. return; } if (keyBindings.HistoryPrevious.Matches(key.ConsoleKeyInfo)) { + // if we're in a wrapped line, we want to allow the user to move up and down within the current input before navigating history. + if (!allowInMultilineStatement && codePane.Cursor.Row > 0) + { + return; + } + int startIndex = -1; if (historyPath.Count == 0) { @@ -128,6 +149,12 @@ public async Task OnKeyUp(KeyPress key, CancellationToken cancellationToken) } else if (keyBindings.HistoryNext.Matches(key.ConsoleKeyInfo)) { + // Symmetric to the up-arrow case above: if we're in a wrapped line, we want to allow the user to move up and down within the current input before navigating history. + if (!allowInMultilineStatement && codePane.Cursor.Row < codePane.WordWrappedLines.Count - 1) + { + return; + } + if (historyPath.Count > 0) { historyPath.Pop(); @@ -145,7 +172,8 @@ public async Task OnKeyUp(KeyPress key, CancellationToken cancellationToken) } else { - historyPath.Clear(); + // Cursor movement and other non-editing keys leave historyPath intact (see the edit detection + // above), so that navigating around a recalled entry doesn't detach us from history. key.Handled = false; } } @@ -181,8 +209,12 @@ bool TryGet(out int historyIndex, Func isMatch) } } - private static void SetContents(CodePane codepane, string contents) + private void SetContents(CodePane codepane, string contents) { + // remember what we navigated to, so a later keypress can tell cursor movement (contents unchanged) + // apart from an edit (contents changed) - see the edit detection in OnKeyUp. + contentsAfterLastNavigation = contents; + if (codepane.Document.Equals(contents)) return; codepane.Document.SetContents(codepane, contents, caret: contents.Length); @@ -204,6 +236,7 @@ internal void Track(CodePane codePane) } } historyPath.Clear(); + contentsAfterLastNavigation = string.Empty; // new prompt starts empty and detached from history this.codePane = codePane; } diff --git a/tests/PrettyPrompt.Tests/HistoryTests.cs b/tests/PrettyPrompt.Tests/HistoryTests.cs index e5b3319..b4256d6 100644 --- a/tests/PrettyPrompt.Tests/HistoryTests.cs +++ b/tests/PrettyPrompt.Tests/HistoryTests.cs @@ -291,6 +291,92 @@ public async Task ReadLine_UpArrow_DoesNotCycleThroughHistory_WhenInMultilineSta Assert.Equal($"b{Environment.NewLine}", result.Text); } + /// + /// https://github.com/waf/CSharpRepl/issues/247 + /// When a single logical line is long enough to word-wrap across multiple display rows, the up-arrow + /// should move the cursor up within those wrapped rows rather than navigating to a previous history + /// entry. History navigation should only kick in once the cursor is already on the first display row. + /// + [Fact] + public async Task ReadLine_UpArrow_MovesCursorWithinWrappedLine_InsteadOfCyclingHistory() + { + var console = ConsoleStub.NewConsole(width: 20); // code area is 18 columns wide after the "> " prompt + var prompt = new Prompt(console: console); + + console.StubInput($"old entry{Enter}"); + await prompt.ReadLineAsync(); + + // a single logical line (no newlines) long enough to wrap across multiple display rows + var wrapped = new string('a', 40); + console.StubInput($"{wrapped}{Enter}"); + await prompt.ReadLineAsync(); + + console.StubInput( + $"{UpArrow}", // loads the wrapped entry; cursor lands at its end, on the last display row + $"{LeftArrow}", // moves the cursor and ends the active history cycle + $"{UpArrow}", // cursor is not on the first display row, so this moves it up within the wrapped + // entry instead of navigating back to "old entry" + $"{Enter}"); + var result = await prompt.ReadLineAsync(); + + Assert.Equal(wrapped, result.Text); + } + + /// + /// Follow-up to https://github.com/waf/CSharpRepl/issues/247 + /// After recalling a multiline history entry and moving the cursor up within it, pressing Down should walk + /// the cursor back down through the entry and then return to the original (empty) prompt, rather than + /// trapping the user on the recalled entry. + /// + [Fact] + public async Task ReadLine_DownArrow_ReturnsToEmptyPrompt_AfterNavigatingWithinRecalledMultilineEntry() + { + var console = ConsoleStub.NewConsole(); + var prompt = new Prompt(console: console); + + console.StubInput($"a{Shift}{Enter}b{Enter}"); // submit the multiline entry "a\nb" + await prompt.ReadLineAsync(); + + console.StubInput( + $"{UpArrow}", // recall "a\nb"; cursor lands on the last display row + $"{UpArrow}", // no older history to cycle to, so this moves the cursor up within the entry + $"{DownArrow}", // move the cursor back down within the entry + $"{DownArrow}", // cursor is on the last row again, so this returns to the empty prompt + $"x{Enter}"); // typing on the now-empty prompt + var result = await prompt.ReadLineAsync(); + + Assert.Equal("x", result.Text); + } + + /// + /// Follow-up to https://github.com/waf/CSharpRepl/issues/247 + /// Moving the cursor around a recalled (word-wrapped) entry must not detach us from history. After + /// navigating up/down/left/right within the wrapped entry, pressing Down from its last row should still + /// return to the original (empty) prompt rather than trapping the user on the recalled entry. + /// + [Fact] + public async Task ReadLine_DownArrow_ReturnsToEmptyPrompt_AfterMovingCursorWithinRecalledWrappedEntry() + { + var console = ConsoleStub.NewConsole(width: 20); // code area is 18 columns wide after the "> " prompt + var prompt = new Prompt(console: console); + + var wrapped = new string('a', 40); // a single logical line that wraps across multiple display rows + console.StubInput($"{wrapped}{Enter}"); + await prompt.ReadLineAsync(); + + console.StubInput( + $"{UpArrow}", // recall the wrapped entry; cursor lands at its end (last display row) + $"{LeftArrow}", // move the cursor (must NOT detach from history) + $"{UpArrow}", // move the cursor up within the wrapped entry + $"{DownArrow}", // move the cursor back down within the entry + $"{RightArrow}", // move the cursor back to the end of the line + $"{DownArrow}", // cursor is on the last row, so this returns to the empty prompt + $"x{Enter}"); // typing on the now-empty prompt + var result = await prompt.ReadLineAsync(); + + Assert.Equal("x", result.Text); + } + /// /// https://github.com/waf/PrettyPrompt/issues/188 ///