Syntax highlighting bug fixes

src/PrettyPrompt/Highlighting/CellRenderer.cs

@@ -54,38 +54,51 @@ public static Row[] ApplyColorToCharacters(IReadOnlyCollection<FormatSpan> highl
         for (int lineIndex = startLine; lineIndex < endLine; lineIndex++)
         {
             WrappedLine line = lines[lineIndex];
-            int lineFullWidthCharacterOffset = 0;
+            // characterPosition coordinates are UTF-16 document offsets (the same units the highlight spans and
+            // selection columns use). A cell, by contrast, is a display column. These two differ whenever a
+            // grapheme cluster is not exactly one char wide AND one column wide:
+            //   CJK '界'      -> 1 char,  2 columns
+            //   emoji '🙃'     -> 2 chars (surrogate pair), 2 columns
+            //   combining 'é' -> 2 chars, 1 column
+            // so we track the cluster's UTF-16 length (lineCharOffset) independently of the cell index.
+            int lineCharOffset = 0;           // UTF-16 offset within the line of the current cell's cluster
+            int currentClusterCharLength = 0; // UTF-16 length of that cluster, captured from its main cell
             var row = new Row(line.Content);
             for (int cellIndex = 0; cellIndex < row.Length; cellIndex++)
             {
                 var cell = row[cellIndex];
-                if (cell.IsContinuationOfPreviousCharacter)
-                    lineFullWidthCharacterOffset++;
+                // A main (non-continuation) cell starts a new cluster: advance past the previous cluster's chars.
+                // Continuation cells belong to the same cluster as their main cell and share its char offset.
+                if (!cell.IsContinuationOfPreviousCharacter)
+                {
+                    lineCharOffset += currentClusterCharLength;
+                    currentClusterCharLength = cell.Text?.Length ?? 1;
+                }
+                int characterPosition = line.StartIndex + lineCharOffset;
 
-                // syntax highlight wrapped lines
+                // syntax highlight wrapped lines (a highlight carried over from the previous line)
                 if (currentHighlight.TryGet(out var previousLineHighlight) &&
                     cellIndex == 0)
                 {
-                    currentHighlight = HighlightSpan(previousLineHighlight, row, cellIndex, previousLineHighlight.Start - line.StartIndex);
+                    currentHighlight = HighlightSpan(previousLineHighlight, row, cellIndex, characterPosition);
                 }
 
                 // get current syntaxt highlight start
-                int characterPosition = line.StartIndex + cellIndex - lineFullWidthCharacterOffset;
                 currentHighlight ??= highlightsLookup.TryGetValue(characterPosition, out var lookupHighlight) ? lookupHighlight : null;
 
                 // syntax highlight based on start
                 if (currentHighlight.TryGet(out var highlight) &&
                     highlight.Contains(characterPosition))
                 {
-                    currentHighlight = HighlightSpan(highlight, row, cellIndex, cellIndex);
+                    currentHighlight = HighlightSpan(highlight, row, cellIndex, characterPosition);
                 }
 
                 // if there's text selected, invert colors to represent the highlight of the selected text.
-                if (selectionStart.Equals(lineIndex, cellIndex - lineFullWidthCharacterOffset)) //start is inclusive
+                if (selectionStart.Equals(lineIndex, lineCharOffset)) //start is inclusive
                 {
                     selectionHighlight = true;
                 }
-                if (selectionEnd.Equals(lineIndex, cellIndex - lineFullWidthCharacterOffset)) //end is exclusive
+                if (selectionEnd.Equals(lineIndex, lineCharOffset)) //end is exclusive
                 {
                     selectionHighlight = false;
                 }
@@ -144,14 +157,22 @@ public static Row[] ApplyColorToCharacters(IReadOnlyCollection<FormatSpan> highl
         return seed;
     }
 
-    private static FormatSpan? HighlightSpan(FormatSpan currentHighlight, Row row, int cellIndex, int endPosition)
+    /// <summary>
+    /// Colors the cells of <paramref name="row"/> from <paramref name="cellIndex"/> forward that fall within
+    /// <paramref name="currentHighlight"/>, stopping once the running UTF-16 document offset reaches the
+    /// highlight's end (or the row ends). <paramref name="charOffset"/> is the UTF-16 offset of the cluster at
+    /// <paramref name="cellIndex"/>.
+    /// </summary>
+    private static FormatSpan? HighlightSpan(FormatSpan currentHighlight, Row row, int cellIndex, int charOffset)
     {
-        var highlightedFullWidthOffset = 0;
         int i;
-        for (i = cellIndex; i < Math.Min(endPosition + currentHighlight.Length + highlightedFullWidthOffset, row.Length); i++)
+        for (i = cellIndex; i < row.Length && charOffset < currentHighlight.End; i++)
         {
-            highlightedFullWidthOffset += row[i].ElementWidth - 1;
             row[i].Formatting = currentHighlight.Formatting;
+            if (!row[i].IsContinuationOfPreviousCharacter)
+            {
+                charOffset += row[i].Text?.Length ?? 1;
+            }
         }
         if (i != row.Length)
         {

src/PrettyPrompt/Rendering/IncrementalRendering.cs

@@ -85,16 +85,24 @@ private static StringBuilder CalculateDiffInternal(Screen currentScreen, Screen
             MoveCursorIfRequired(diff, previousCoordinate, cellCoordinate);
             previousCoordinate = cellCoordinate;
 
+            // A newline cell paints no glyph at its own column, so - exactly like a null/erased cell - we have to
+            // emit a clearing space there to overwrite whatever character was rendered at that column before, and
+            // only then the newline to move down. The newline's OWN formatting is irrelevant (it draws nothing),
+            // and a multi-line string literal's highlight span legitimately colors the newline cell. Gating the
+            // clear on the cell's formatting being default (as we do for real characters) skipped a colored
+            // newline and left the old character on screen as a "ghost" See https://github.com/waf/CSharpRepl/issues/411.
+            bool currentCellIsNewLine = currentCell is not null && currentCell.Text == "\n";
+
             // handle when we're erasing characters/formatting from the previously rendered screen.
-            if (currentCell is null || currentCell.Formatting.IsDefault)
+            if (currentCell is null || currentCell.Formatting.IsDefault || currentCellIsNewLine)
             {
                 if (!currentFormatRun.IsDefault)
                 {
                     diff.Append(Reset);
                     currentFormatRun = ConsoleFormat.None;
                 }
 
-                if (currentCell?.Text is null || currentCell.Text == "\n")
+                if (currentCell?.Text is null || currentCellIsNewLine)
                 {
                     diff.Append(' ');
                     UpdateCoordinateFromCursorMove(previousScreen, ansiCoordinate, diff, ref previousCoordinate, currentCell);
@@ -103,6 +111,11 @@ private static StringBuilder CalculateDiffInternal(Screen currentScreen, Screen
                     {
                         continue;
                     }
+
+                    // The clearing space is written; now emit the newline itself to move the cursor down.
+                    diff.Append(currentCell.Text);
+                    UpdateCoordinateFromNewLine(ref previousCoordinate);
+                    continue;
                 }
             }
 
@@ -123,17 +136,10 @@ private static StringBuilder CalculateDiffInternal(Screen currentScreen, Screen
                 diff.Append(currentCell.Text);
             }
 
-            // writing to the console will automatically move the cursor.
-            // update our internal tracking so we calculate the least
-            // amount of movement required for the next character.
-            if (currentCell.Text == "\n")
-            {
-                UpdateCoordinateFromNewLine(ref previousCoordinate);
-            }
-            else
-            {
-                UpdateCoordinateFromCursorMove(currentScreen, ansiCoordinate, diff, ref previousCoordinate, currentCell);
-            }
+            // writing to the console will automatically move the cursor. update our internal tracking so we
+            // calculate the least amount of movement required for the next character. (Newline cells never reach
+            // here - they're written and their coordinate advanced in the erase block above and then `continue`.)
+            UpdateCoordinateFromCursorMove(currentScreen, ansiCoordinate, diff, ref previousCoordinate, currentCell);
         }
 
         if (!currentFormatRun.IsDefault)

tests/PrettyPrompt.Tests/IncrementalRenderingTests.cs

@@ -0,0 +1,148 @@
+#region License Header
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+#endregion
+
+using System;
+using System.Collections.Generic;
+using System.Text;
+using PrettyPrompt.Consoles;
+using PrettyPrompt.Highlighting;
+using PrettyPrompt.Rendering;
+using Xunit;
+
+namespace PrettyPrompt.Tests;
+
+public class IncrementalRenderingTests
+{
+    /// <summary>
+    /// Regression test for CSharpRepl issue #411 (https://github.com/waf/CSharpRepl/issues/411).
+    /// </summary>
+    [Fact]
+    public void CalculateDiff_ColoredNewlineShiftedLeft_ErasesGhostCharacterAtNewlineColumn()
+    {
+        // The string-literal highlight color. Any non-default format reproduces the bug; the point is only that
+        // the newline cell is NOT formatted with the default format.
+        var stringColor = new ConsoleFormat(Foreground: AnsiColor.Red);
+        var ansiCoordinate = new ConsoleCoordinate(1, 1);
+
+        // BEFORE Shift+Enter: a single line `("""`+`)` where the unterminated raw string literal colors the
+        // quotes and the trailing ')'.
+        var before = new Screen(
+            width: 20, height: 5, new ConsoleCoordinate(0, 5),
+            new ScreenArea(ConsoleCoordinate.Zero, new[] { CodeRow(("(", default), ("\"\"\"", stringColor), (")", stringColor)) }, TruncateToScreenHeight: false));
+
+        // AFTER Shift+Enter at the caret between `"""` and `)`: the ')' moves to the next line and the raw string
+        // literal now spans the newline, so the newline cell is colored too.
+        var after = new Screen(
+            width: 20, height: 5, new ConsoleCoordinate(1, 0),
+            new ScreenArea(ConsoleCoordinate.Zero, new[]
+            {
+                CodeRow(("(", default), ("\"\"\"", stringColor), ("\n", stringColor)),
+                CodeRow((")", stringColor)),
+            }, TruncateToScreenHeight: false));
+
+        // Mimic the way a real console works: first the full draw of `before`, then the
+        // incremental diff that turns `before` into `after`. What remains on the (emulated) screen is what the
+        // user actually sees.
+        var terminal = new FakeTerminal(ansiCoordinate);
+        terminal.Apply(IncrementalRendering.CalculateDiff(before, new Screen(0, 0, ConsoleCoordinate.Zero), ansiCoordinate));
+        terminal.Apply(IncrementalRendering.CalculateDiff(after, before, ansiCoordinate));
+
+        Assert.Equal("(\"\"\"", terminal.ReadRow(row: 1, width: 20)); // first line ends at the quotes...
+        Assert.Equal(")", terminal.ReadRow(row: 2, width: 20));       // ...the ')' is only on the second line
+        Assert.DoesNotContain(")", terminal.ReadRow(row: 1, width: 20)); // and never lingers as a ghost on the first
+    }
+
+    private static Row CodeRow(params (string text, ConsoleFormat format)[] segments)
+    {
+        var row = new Row(capacity: 8);
+        foreach (var (text, format) in segments)
+        {
+            row.Add(text, format);
+        }
+        return row;
+    }
+
+    /// <summary>
+    /// A minimal ANSI terminal that records the visible character at each screen coordinate. It understands only
+    /// what <see cref="IncrementalRendering"/> emits: printable text, '\n', the cursor moves (CSI A/B/C/D/G) and
+    /// color/clear sequences (which it ignores, as they don't change which character occupies a cell). The '\n'
+    /// handling mirrors <c>IncrementalRendering.UpdateCoordinateFromNewLine</c> so the emulated cursor tracks the
+    /// renderer's own assumption on each platform.
+    /// </summary>
+    private sealed class FakeTerminal
+    {
+        private const char Escape = (char)27;
+
+        private readonly Dictionary<(int row, int col), char> grid = new();
+        private readonly int originColumn;
+        private int row;
+        private int col;
+
+        public FakeTerminal(ConsoleCoordinate origin)
+        {
+            row = origin.Row;
+            col = origin.Column;
+            originColumn = origin.Column;
+        }
+
+        public void Apply(string ansi)
+        {
+            int i = 0;
+            while (i < ansi.Length)
+            {
+                char c = ansi[i];
+                if (c == Escape && i + 1 < ansi.Length && ansi[i + 1] == '[')
+                {
+                    int paramStart = i + 2;
+                    int j = paramStart;
+                    while (j < ansi.Length && !char.IsLetter(ansi[j])) j++;
+                    if (j < ansi.Length)
+                    {
+                        ApplyControlSequence(ansi[j], ansi.Substring(paramStart, j - paramStart));
+                    }
+                    i = j + 1;
+                }
+                else if (c == '\n')
+                {
+                    row++;
+                    // Matches UpdateCoordinateFromNewLine: on non-Windows a '\n' also returns to the first column.
+                    if (!OperatingSystem.IsWindows()) col = originColumn;
+                    i++;
+                }
+                else
+                {
+                    grid[(row, col)] = c;
+                    col++;
+                    i++;
+                }
+            }
+        }
+
+        private void ApplyControlSequence(char command, string parameters)
+        {
+            int.TryParse(parameters.Split(';')[0], out int n);
+            switch (command)
+            {
+                case 'A': row -= n; break;
+                case 'B': row += n; break;
+                case 'C': col += n; break;
+                case 'D': col -= n; break;
+                case 'G': col = n; break;
+                // 'm' (color) and 'K'/'J' (clear) don't change which character occupies a cell here.
+            }
+        }
+
+        public string ReadRow(int row, int width)
+        {
+            var sb = new StringBuilder();
+            for (int x = originColumn; x < originColumn + width; x++)
+            {
+                sb.Append(grid.TryGetValue((row, x), out var ch) ? ch : ' ');
+            }
+            return sb.ToString().TrimEnd();
+        }
+    }
+}

tests/PrettyPrompt.Tests/SyntaxHighlightingTests.cs

@@ -106,4 +106,31 @@ public async Task ReadLine_CJKCharacters_SyntaxHighlight()
             str => str.Contains("ado" + Reset)
         );
     }
+
+    [Theory]
+    [InlineData("var x = \"abcdefg🙃hijklmnop\";")] // 🙃 (U+1F643): surrogate pair -> 2 chars, 2 columns
+    [InlineData("var x = \"ábcdefghij\";")]    // a + combining acute -> 2 chars, 1 column
+    public void ApplyColorToCharacters_HighlightEndingAfterMultiCharCluster_DoesNotBleedOntoNextCharacter(string text)
+    {
+        // Highlight spans use UTF-16 offsets, so this string-literal span ends exactly before the trailing ';'.
+        // When a grapheme cluster inside the string spans more than one UTF-16 char (a surrogate-pair emoji, or a
+        // base char + combining mark), the renderer must measure the span end in chars - not cells/clusters - or
+        // the string color bleeds onto the ';'. Regression test for the CSharpRepl emoji highlighting bug.
+        int stringStart = text.IndexOf('"');
+        int stringLength = text.LastIndexOf('"') - stringStart + 1;
+        var highlight = new FormatSpan(stringStart, stringLength, AnsiColor.Red);
+
+        var rows = CellRenderer.ApplyColorToCharacters(new[] { highlight }, text, textWidth: 200);
+        var row = rows[0];
+
+        int semicolon = -1;
+        for (int i = 0; i < row.Length; i++)
+        {
+            if (row[i].Text == ";") { semicolon = i; break; }
+        }
+        Assert.True(semicolon > 0, "expected to find a ';' cell");
+
+        Assert.Equal(AnsiColor.Red, row[semicolon - 1].Formatting.Foreground); // closing '"' is highlighted
+        Assert.Null(row[semicolon].Formatting.Foreground);                     // the ';' must NOT be (the bug)
+    }
 }