Fix history next/previous when on long lines that wrap

src/PrettyPrompt/History/HistoryLog.cs

@@ -41,6 +41,12 @@ internal sealed class HistoryLog : IKeyPressHandler
     /// </summary>
     private readonly Stack<int> historyPath = new();
 
+    /// <summary>
+    /// 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 <see cref="historyPath"/>.
+    /// </summary>
+    private string contentsAfterLastNavigation = string.Empty;
+
     private bool historyEntryWasUsedLastTime;
 
     /// <summary>
@@ -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<string, string, bool> 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;
     }
 

tests/PrettyPrompt.Tests/HistoryTests.cs

@@ -291,6 +291,92 @@ public async Task ReadLine_UpArrow_DoesNotCycleThroughHistory_WhenInMultilineSta
         Assert.Equal($"b{Environment.NewLine}", result.Text);
     }
 
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [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);
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [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);
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    [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);
+    }
+
     /// <summary>
     /// https://github.com/waf/PrettyPrompt/issues/188
     /// </summary>