diff --git a/PSReadLine/ReadLine.cs b/PSReadLine/ReadLine.cs index da890bbb..d075f23a 100644 --- a/PSReadLine/ReadLine.cs +++ b/PSReadLine/ReadLine.cs @@ -784,6 +784,7 @@ private void Initialize(Runspace runspace, EngineIntrinsics engineIntrinsics) _parseErrors = null; _inputAccepted = false; _initialX = _console.CursorLeft; + _initialPromptCells = _initialX; _initialY = _console.CursorTop; _initialForeground = _console.ForegroundColor; _initialBackground = _console.BackgroundColor; @@ -1105,6 +1106,7 @@ public static void InvokePrompt(ConsoleKeyInfo? key = null, object arg = null) console.Write(newPrompt); _singleton._initialX = console.CursorLeft; + _singleton._initialPromptCells = _singleton._initialX; _singleton._initialY = console.CursorTop; _singleton._previousRender = _initialPrevRender; _singleton._previousRender.UpdateConsoleInfo(console); diff --git a/PSReadLine/Render.cs b/PSReadLine/Render.cs index 253f7fbe..fd8ce1e3 100644 --- a/PSReadLine/Render.cs +++ b/PSReadLine/Render.cs @@ -166,6 +166,20 @@ struct LineInfoForRendering }; private int _initialX; private int _initialY; + + /// + /// The width, in buffer cells, of the last logical line of the prompt, measured from column 0 + /// of the physical line where that logical line starts. + /// This does not depend on the buffer width, whereas '_initialX' is the column of the same + /// point at the current buffer width, and hence is only ever this value modulo that width. + /// It is what we believe the anchor's column to be after a buffer width change, and it is only + /// a belief: it is seeded from '_initialX', so it is itself already reduced whenever 'ReadLine' + /// starts on a buffer narrower than the prompt. 'RecomputeInitialCoordsFromCursor' checks it + /// against the physical cursor, takes a column the cursor agrees with when it disagrees, and + /// falls back to it when the cursor cannot tell the columns apart. + /// + private int _initialPromptCells; + private bool _waitingToRender; private bool _handlePotentialResizing; @@ -885,6 +899,7 @@ private void CalculateWhereAndWhatToRender(bool cursorMovedToInitialPos, RenderD } _initialX = _console.CursorLeft; + _initialPromptCells = _initialX; _initialY = _console.CursorTop; _previousRender = _initialPrevRender; } @@ -1244,6 +1259,7 @@ private void RecomputeInitialCoords(bool isTextBufferUnchanged) } _initialX = _console.CursorLeft; + _initialPromptCells = _initialX; _initialY = _console.CursorTop; _previousRender = _initialPrevRender; } @@ -1257,15 +1273,16 @@ private void RecomputeInitialCoords(bool isTextBufferUnchanged) // The '_buffer' and '_current' still reflects what has been rendered on the screen, // so we can use them to re-calculate the initial coordinates in this case. - // Recompute X from the buffer width: - _initialX %= _console.BufferWidth; - - // Recompute Y from the cursor - _initialY = 0; - // Calculate the new cursor position when assuming '_initialY' is at line 0. - var pt = ConvertOffsetToPoint(_current); - // Update '_initialY' based on the difference from the actual current cursor position after the resize. - _initialY = _console.CursorTop - pt.Y; + // Recompute both coordinates from the cursor. '_buffer' and '_current' say where the + // cursor is relative to the initial coordinates, and the console says where the cursor + // is on the screen; the difference between the two is the initial coordinates. + RecomputeInitialCoordsFromCursor(column => + { + // Put the anchor at 'column' of line 0 and render the input from there. + _initialX = column; + _initialY = 0; + return ConvertOffsetToPoint(_current); + }); } else { @@ -1293,21 +1310,98 @@ private void RecomputeInitialCoords(bool isTextBufferUnchanged) throw new InvalidOperationException(message); } - // Recompute X from the buffer width: - _initialX %= _console.BufferWidth; - - // Recompute Y from the cursor - _initialY = 0; - // Now, use the new initial coordinates, new buffer width, and the rendering data offset to calculate - // the new cursor position when assuming '_initialY' is at line 0. - Point pt = ConvertRenderDataOffsetToPoint(_initialX, _initialY, _console.BufferWidth, _previousRender, offset); - // Update '_initialY' based on the difference from the actual current cursor position after the resize. + // Recompute both coordinates from the cursor, the same way as above, except that the + // rendering data offset stands in for '_buffer' and '_current'. // This is based on the assumption that the cursor is still pointing to the same character after resizing, // or at least pointing to the physical line where the same character is located after resizing. // However, that assumption is not always guaranteed in Windows Terminal, see the issue: // https://github.com/microsoft/terminal/issues/10848, and // https://github.com/microsoft/terminal/issues/10868 - _initialY = _console.CursorTop - pt.Y; + RecomputeInitialCoordsFromCursor( + column => ConvertRenderDataOffsetToPoint(column, 0, _console.BufferWidth, _previousRender, offset)); + } + } + + /// + /// Recompute the initial coordinates - the cell the edit line is anchored at - after the + /// buffer width changed, from where the console now says the cursor is. + /// + /// + /// Where the cursor would be drawn if the anchor were at the given column of line 0. That is + /// the rendering of the current input, so its distance from the anchor is the display cell + /// offset from the anchor to the cursor. + /// + /// + /// The anchor cannot be observed after a resize: the terminal reflowed the screen and said + /// nothing about where it moved the prompt to. The cursor can be observed, and the terminal + /// moved the two together, so the anchor is at the column that would put the cursor where the + /// console says the cursor is, and at the row that many physical lines above the cursor's. + /// + /// With an empty input this makes the anchor the cursor, which is exactly what capturing the + /// initial coordinates at the start of 'ReadLine' would have given had 'ReadLine' been entered + /// at the new width - including the case where the prompt is wider than the buffer, which is + /// the case no arithmetic on '_initialX' can recover, because '_initialX' is by then already + /// the prompt's cell width reduced modulo the width it was captured at. + /// + /// The cursor does not always tell the columns apart. A newline in the input moves the + /// rendering to the continuation prompt's column no matter where the anchor is, so once the + /// cursor is past one, every column agrees with it; and a double width character pushed whole + /// onto the next physical line leaves a cell of slack, so two neighbouring columns can agree. + /// '_initialPromptCells' is the belief we already hold about the column, so it is where the + /// search starts and the answer whenever the cursor agrees with it - which makes this a strict + /// refinement of deriving the column from the prompt's cell width alone, and leaves the + /// behaviour of every input the cursor says nothing about unchanged. + /// + private void RecomputeInitialCoordsFromCursor(Func cursorPointFrom) + { + int bufferWidth = _console.BufferWidth; + int cursorLeft = _console.CursorLeft; + int cursorTop = _console.CursorTop; + + int believedX = _initialPromptCells % bufferWidth; + Point believed = cursorPointFrom(believedX); + + int newX = believedX; + int newY = cursorTop - believed.Y; + + if (believed.X != cursorLeft || newY < 0) + { + // The belief is not consistent with the cursor, so take the nearest column that is. + // A cursor above the anchor is not a state we can be in, so a column that implies one + // is no more of an answer than a column that puts the cursor elsewhere entirely. + for (int delta = 1; delta < bufferWidth; delta++) + { + if (TryColumn(believedX - delta) || TryColumn(believedX + delta)) + { + break; + } + } + } + + _initialX = newX; + _initialY = newY; + + // '_initialPromptCells' is deliberately left as it is. What it holds over '_initialX' is + // how many physical lines the prompt spans at the width it was captured at, and the cursor + // never reveals that, so rewriting it here from a column observed at a different width + // would mix two units and lose the cases it does answer correctly today. + + bool TryColumn(int column) + { + if (column < 0 || column >= bufferWidth) + { + return false; + } + + Point candidate = cursorPointFrom(column); + if (candidate.X != cursorLeft || cursorTop < candidate.Y) + { + return false; + } + + newX = column; + newY = cursorTop - candidate.Y; + return true; } } diff --git a/test/ResizingTest.cs b/test/ResizingTest.cs index a02ed783..bedb1a1c 100644 --- a/test/ResizingTest.cs +++ b/test/ResizingTest.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Reflection; +using System.Text; using Microsoft.PowerShell; using Newtonsoft.Json; using Xunit; @@ -182,5 +183,325 @@ public void PhysicalLineCountMethod_ShouldWork() } } } + + private static FieldInfo GetInstanceField(string name) + { + return typeof(PSConsoleReadLine).GetField(name, BindingFlags.Instance | BindingFlags.NonPublic); + } + + /// + /// Drive 'RecomputeInitialCoords' through a chain of buffer widths and check that the anchor + /// it recovers at each width is the one the terminal's reflow put on the screen. + /// + /// + /// The cell width of the prompt's last logical line. The prompt is taken to start at column 0 + /// of a physical line, so at every buffer width the anchor is at row 'promptCells / width' and + /// column 'promptCells % width'. That is the only property of the prompt that matters here. + /// + /// The input as it stands on the screen when the buffer width changes. + /// Where '_current' points into that input. + /// + /// The width 'ReadLine' was entered at, followed by the widths the buffer is changed to. + /// + /// + /// The cursor the test console reports at each width is the one a terminal that reflowed the + /// screen would report: the anchor plus the rendering of the input up to '_current'. The + /// rendering is the forward direction of the very calculation under test, which is what makes + /// this a test of the inverse - given a rendering and where it ended up on the screen, where + /// does the anchor have to be - and not a restatement of it. + /// + private void AssertAnchorIsRecoveredAcrossWidths( + int promptCells, + string input, + int cursorOffset, + int[] bufferWidths) + { + const int bufferHeight = 200; + + PSConsoleReadLine instance = GetPSConsoleReadLineSingleton(); + FieldInfo consoleField = GetInstanceField("_console"); + FieldInfo bufferField = GetInstanceField("_buffer"); + FieldInfo currentField = GetInstanceField("_current"); + FieldInfo initialXField = GetInstanceField("_initialX"); + FieldInfo initialYField = GetInstanceField("_initialY"); + FieldInfo initialPromptCellsField = GetInstanceField("_initialPromptCells"); + FieldInfo previousRenderField = GetInstanceField("_previousRender"); + FieldInfo handlePotentialResizingField = GetInstanceField("_handlePotentialResizing"); + MethodInfo recomputeInitialCoords = typeof(PSConsoleReadLine) + .GetMethod("RecomputeInitialCoords", BindingFlags.Instance | BindingFlags.NonPublic); + + object savedConsole = consoleField.GetValue(instance); + object savedBuffer = bufferField.GetValue(instance); + object savedCurrent = currentField.GetValue(instance); + object savedInitialX = initialXField.GetValue(instance); + object savedInitialY = initialYField.GetValue(instance); + object savedInitialPromptCells = initialPromptCellsField.GetValue(instance); + object savedPreviousRender = previousRenderField.GetValue(instance); + + try + { + bufferField.SetValue(instance, new StringBuilder(input)); + currentField.SetValue(instance, cursorOffset); + + // What 'ReadLine' captured at the width it was entered at. '_initialX' is the column + // the console reported, which is the prompt's cell width reduced modulo that width, + // and '_initialPromptCells' is seeded from it - so the seed is the prompt's true cell + // width only when the prompt fitted the buffer it started on. + int entryWidth = bufferWidths[0]; + int heldX = promptCells % entryWidth; + int heldY = promptCells / entryWidth; + initialXField.SetValue(instance, heldX); + initialYField.SetValue(instance, heldY); + initialPromptCellsField.SetValue(instance, heldX); + + RenderData previousRender = new() + { + lines = new[] { new RenderedLineData(line: input, isFirstLogicalLine: true) } + }; + + for (int i = 1; i < bufferWidths.Length; i++) + { + int bufferWidth = bufferWidths[i]; + TestConsole console = new(_, bufferWidth, bufferHeight); + consoleField.SetValue(instance, console); + + // Where the reflow left the anchor, and hence where it left the cursor. + int expectedX = promptCells % bufferWidth; + int expectedY = promptCells / bufferWidth; + initialXField.SetValue(instance, expectedX); + initialYField.SetValue(instance, expectedY); + Point cursor = instance.ConvertOffsetToPoint(cursorOffset); + console.CursorLeft = cursor.X; + console.CursorTop = cursor.Y; + + // The coordinates PSReadLine actually holds are the ones from the previous width. + initialXField.SetValue(instance, heldX); + initialYField.SetValue(instance, heldY); + previousRender.initialY = heldY; + previousRenderField.SetValue(instance, previousRender); + handlePotentialResizingField.SetValue(instance, true); + + recomputeInitialCoords.Invoke(instance, new object[] { true }); + + heldX = (int)initialXField.GetValue(instance); + heldY = (int)initialYField.GetValue(instance); + Assert.True( + expectedX == heldX && expectedY == heldY, + $"prompt of {promptCells} cells, buffer width {bufferWidth}: the anchor was " + + $"recovered at ({heldX}, {heldY}) but the reflow left it at ({expectedX}, {expectedY})"); + + // The render data now describes the buffer as it was before the next change. + previousRender.UpdateConsoleInfo(console); + } + } + finally + { + consoleField.SetValue(instance, savedConsole); + bufferField.SetValue(instance, savedBuffer); + currentField.SetValue(instance, savedCurrent); + initialXField.SetValue(instance, savedInitialX); + initialYField.SetValue(instance, savedInitialY); + initialPromptCellsField.SetValue(instance, savedInitialPromptCells); + previousRenderField.SetValue(instance, savedPreviousRender); + } + } + + [Fact] + public void RecomputeInitialCoords_ShouldRecoverInitialXWhenBufferGetsWider() + { + // The column of the anchor is the width of the prompt reduced modulo the buffer width, so + // a prompt of 36 cells walked through the buffer widths below has to give 36, 1, 36, 11 + // and 36 in turn. Reducing the column in place on each change gives the right answer only + // for the first one, because it discards how many physical lines the prompt spans and the + // column can then no longer be recovered. + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 36, + input: "", + cursorOffset: 0, + bufferWidths: new[] { 100, 35, 60, 25, 100 }); + } + + [Fact] + public void RecomputeInitialCoords_ShouldRecoverInitialXWhenThePromptDidNotFitTheInitialBuffer() + { + // 'ReadLine' is entered on a buffer narrower than the prompt, so the column the console + // reports - and therefore everything derived from it - is already reduced: 82 % 62 == 20. + // No arithmetic on that 20 can produce 82 again. The cursor can, because the terminal + // reflowed the prompt and the cursor together and the input is empty, which puts the + // cursor on the anchor itself. + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 82, + input: "", + cursorOffset: 0, + bufferWidths: new[] { 62, 82 }); + + // The same, then on to widths the prompt does and does not fit, in both directions. + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 82, + input: "", + cursorOffset: 0, + bufferWidths: new[] { 62, 100, 70, 120, 62 }); + } + + [Fact] + public void RecomputeInitialCoords_ShouldRecoverInitialXWithTextOnTheInputLine() + { + // The cursor is no longer on the anchor, so recovering the anchor means subtracting the + // rendering of the input from it. + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 82, + input: "Get-ChildItem", + cursorOffset: 13, + bufferWidths: new[] { 62, 82 }); + + // Narrow to wide, with an input long enough to wrap at every width in the chain. + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 82, + input: "Get-ChildItem -Path . -Recurse | Where-Object { $_.Length -gt 1024 }", + cursorOffset: 67, + bufferWidths: new[] { 62, 100, 130 }); + + // Wide to narrow, and with the cursor inside the input rather than at its end. + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 36, + input: "Get-ChildItem -Path . -Recurse | Where-Object { $_.Length -gt 1024 }", + cursorOffset: 30, + bufferWidths: new[] { 120, 60, 40 }); + } + + [Fact] + public void RecomputeInitialCoords_ShouldKeepWorkingWhenThePromptFitsTheBuffer() + { + // The prompt fitted the buffer 'ReadLine' was entered on, so the belief about its cell + // width was never damaged and the cursor has to agree with it at every width. + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 12, + input: "", + cursorOffset: 0, + bufferWidths: new[] { 80, 40, 120, 20, 80 }); + + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 12, + input: "Get-ChildItem -Path .", + cursorOffset: 21, + bufferWidths: new[] { 80, 40, 120, 20, 80 }); + } + + [Fact] + public void RecomputeInitialCoords_ShouldRecoverInitialXWithAMultiLineInput() + { + // A newline moves the rendering to the continuation prompt's column no matter where the + // anchor is, so past one the cursor says nothing about the anchor's column: taking the + // cursor's own column as the offset to subtract would put the anchor at column 0 and draw + // the first logical line over the prompt. The belief about the prompt's cell width is the + // answer here, and the cursor must be read as agreeing with it rather than replacing it. + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 30, + input: "Get-ChildItem |\nForEach-Object { $_.Name }", + cursorOffset: 41, + bufferWidths: new[] { 80, 60, 25, 100 }); + + // With the cursor still on the first logical line, where the column is observable again. + AssertAnchorIsRecoveredAcrossWidths( + promptCells: 30, + input: "Get-ChildItem |\nForEach-Object { $_.Name }", + cursorOffset: 10, + bufferWidths: new[] { 80, 60, 25, 100 }); + } + + [Fact] + public void RecomputeInitialCoords_ShouldRecoverInitialXFromRenderDataWhenTheInputChanged() + { + // The other half of 'RecomputeInitialCoords': the input has changed since it was last + // rendered - 'Escape' cleared it after the resize, say - so the anchor has to be recovered + // from the previous rendering rather than from '_buffer' and '_current'. + const int promptCells = 82; + const int entryWidth = 62; + const int newWidth = 82; + const int bufferHeight = 200; + const string rendered = "Get-ChildItem -Path . -Recurse"; + + PSConsoleReadLine instance = GetPSConsoleReadLineSingleton(); + FieldInfo consoleField = GetInstanceField("_console"); + FieldInfo bufferField = GetInstanceField("_buffer"); + FieldInfo currentField = GetInstanceField("_current"); + FieldInfo initialXField = GetInstanceField("_initialX"); + FieldInfo initialYField = GetInstanceField("_initialY"); + FieldInfo initialPromptCellsField = GetInstanceField("_initialPromptCells"); + FieldInfo previousRenderField = GetInstanceField("_previousRender"); + FieldInfo handlePotentialResizingField = GetInstanceField("_handlePotentialResizing"); + MethodInfo recomputeInitialCoords = typeof(PSConsoleReadLine) + .GetMethod("RecomputeInitialCoords", BindingFlags.Instance | BindingFlags.NonPublic); + + object savedConsole = consoleField.GetValue(instance); + object savedBuffer = bufferField.GetValue(instance); + object savedCurrent = currentField.GetValue(instance); + object savedInitialX = initialXField.GetValue(instance); + object savedInitialY = initialYField.GetValue(instance); + object savedInitialPromptCells = initialPromptCellsField.GetValue(instance); + object savedPreviousRender = previousRenderField.GetValue(instance); + + try + { + int heldX = promptCells % entryWidth; + int heldY = promptCells / entryWidth; + + // The input was cleared after the resize, which is what makes this the other branch. + bufferField.SetValue(instance, new StringBuilder()); + currentField.SetValue(instance, 0); + initialXField.SetValue(instance, heldX); + initialYField.SetValue(instance, heldY); + initialPromptCellsField.SetValue(instance, heldX); + + // The rendering as it stood at the entry width, with the cursor at the end of it. + RenderData previousRender = new() + { + lines = new[] { new RenderedLineData(rendered, isFirstLogicalLine: true) }, + bufferWidth = entryWidth, + bufferHeight = bufferHeight, + initialY = heldY, + }; + Point oldCursor = instance.ConvertRenderDataOffsetToPoint( + heldX, heldY, entryWidth, previousRender, new RenderDataOffset(0, int.MaxValue)); + previousRender.cursorLeft = oldCursor.X; + previousRender.cursorTop = oldCursor.Y; + previousRenderField.SetValue(instance, previousRender); + + // Where the reflow left the anchor, and the cursor that follows from it. + int expectedX = promptCells % newWidth; + int expectedY = promptCells / newWidth; + RenderDataOffset offset = instance.ConvertPointToRenderDataOffset(heldX, heldY, previousRender); + Assert.NotEqual(-1, offset.LogicalLineIndex); + Point newCursor = instance.ConvertRenderDataOffsetToPoint( + expectedX, expectedY, newWidth, previousRender, offset); + + TestConsole console = new(_, newWidth, bufferHeight) + { + CursorLeft = newCursor.X, + CursorTop = newCursor.Y, + }; + consoleField.SetValue(instance, console); + handlePotentialResizingField.SetValue(instance, true); + + recomputeInitialCoords.Invoke(instance, new object[] { false }); + + int initialX = (int)initialXField.GetValue(instance); + int initialY = (int)initialYField.GetValue(instance); + Assert.True( + expectedX == initialX && expectedY == initialY, + $"buffer width {newWidth}: the anchor was recovered at ({initialX}, {initialY}) " + + $"but the reflow left it at ({expectedX}, {expectedY})"); + } + finally + { + consoleField.SetValue(instance, savedConsole); + bufferField.SetValue(instance, savedBuffer); + currentField.SetValue(instance, savedCurrent); + initialXField.SetValue(instance, savedInitialX); + initialYField.SetValue(instance, savedInitialY); + initialPromptCellsField.SetValue(instance, savedInitialPromptCells); + previousRenderField.SetValue(instance, savedPreviousRender); + } + } } }