From 8f8cdb6556296361c8041e8731a8080354af1378 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 10 May 2022 10:16:50 -0700 Subject: [PATCH 1/7] wip --- .../FormatAndOutput/common/ComplexWriter.cs | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs index 3adb5a799c2..748b3271de2 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs @@ -6,8 +6,10 @@ using System.Collections.ObjectModel; using System.Collections.Specialized; using System.Globalization; +using System.Management.Automation; using System.Management.Automation.Internal; using System.Text; +using System.Text.RegularExpressions; namespace Microsoft.PowerShell.Commands.Internal.Format { @@ -353,27 +355,45 @@ static StringManipulationHelper() private static IEnumerable GetWords(string s) { StringBuilder sb = new StringBuilder(); - GetWordsResult result = new GetWordsResult(); + StringBuilder vtSeqs = null; + Dictionary ansiRanges = null; + + var valueStrDec = new ValueStringDecorated(s); + if (valueStrDec.IsDecorated) + { + vtSeqs = new StringBuilder(); + ansiRanges = new Dictionary(); + foreach (Match match in ValueStringDecorated.AnsiRegex.Matches(s)) + { + ansiRanges.Add(match.Index, match.Length); + } + } for (int i = 0; i < s.Length; i++) { // Soft hyphen = \u00AD - Should break, and add a hyphen if needed. If not needed for a break, hyphen should be absent if (s[i] == ' ' || s[i] == '\t' || s[i] == s_softHyphen) { - result.Word = sb.ToString(); - sb.Clear(); - result.Delim = new string(s[i], 1); + if (valueStrDec.IsDecorated && !sb.EndsWith(PSStyle.Instance.Reset)) + { + sb.Append(PSStyle.Instance.Reset); + } + var result = new GetWordsResult() { Word = sb.ToString(), Delim = new string(s[i], 1) }; + sb.Clear(); yield return result; } // Non-breaking space = \u00A0 - ideally shouldn't wrap // Hard hyphen = \u2011 - Should not break else if (s[i] == s_hardHyphen || s[i] == s_nonBreakingSpace) { - result.Word = sb.ToString(); - sb.Clear(); - result.Delim = string.Empty; + if (valueStrDec.IsDecorated && !sb.EndsWith(PSStyle.Instance.Reset)) + { + sb.Append(PSStyle.Instance.Reset); + } + var result = new GetWordsResult() { Word = sb.ToString(), Delim = string.Empty }; + sb.Clear(); yield return result; } else @@ -382,10 +402,7 @@ private static IEnumerable GetWords(string s) } } - result.Word = sb.ToString(); - result.Delim = string.Empty; - - yield return result; + yield return new GetWordsResult() { Word = sb.ToString(), Delim = string.Empty }; } internal static StringCollection GenerateLines(DisplayCells displayCells, string val, int firstLineLen, int followingLinesLen) From 327a6bdf570b6af20cc037a6acb1415b26a13be6 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 11 May 2022 11:00:54 -0700 Subject: [PATCH 2/7] Finish the fix and add tests --- .../FormatAndOutput/common/ComplexWriter.cs | 219 ++++++++++++------ .../FormatAndOutput/common/ILineOutput.cs | 5 +- .../FormatAndOutput/common/ListWriter.cs | 34 +-- .../engine/Formatting/PSStyle.Tests.ps1 | 38 +++ 4 files changed, 214 insertions(+), 82 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs index 748b3271de2..f3164ee35d0 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs @@ -148,7 +148,7 @@ private void WriteToScreen() int indentationAbsoluteValue = (firstLineIndentation > 0) ? firstLineIndentation : -firstLineIndentation; if (indentationAbsoluteValue >= usefulWidth) { - // valu too big, we reset it to zero + // value too big, we reset it to zero firstLineIndentation = 0; } @@ -356,44 +356,61 @@ private static IEnumerable GetWords(string s) { StringBuilder sb = new StringBuilder(); StringBuilder vtSeqs = null; - Dictionary ansiRanges = null; + Dictionary vtRanges = null; var valueStrDec = new ValueStringDecorated(s); if (valueStrDec.IsDecorated) { vtSeqs = new StringBuilder(); - ansiRanges = new Dictionary(); + vtRanges = new Dictionary(); foreach (Match match in ValueStringDecorated.AnsiRegex.Matches(s)) { - ansiRanges.Add(match.Index, match.Length); + vtRanges.Add(match.Index, match.Length); } } + bool wordHasVtSeqs = false; for (int i = 0; i < s.Length; i++) { - // Soft hyphen = \u00AD - Should break, and add a hyphen if needed. If not needed for a break, hyphen should be absent - if (s[i] == ' ' || s[i] == '\t' || s[i] == s_softHyphen) + if (vtRanges?.TryGetValue(i, out int len) == true) { - if (valueStrDec.IsDecorated && !sb.EndsWith(PSStyle.Instance.Reset)) - { - sb.Append(PSStyle.Instance.Reset); - } + var vtSpan = s.AsSpan(i, len); + sb.Append(vtSpan); + vtSeqs.Append(vtSpan); - var result = new GetWordsResult() { Word = sb.ToString(), Delim = new string(s[i], 1) }; - sb.Clear(); - yield return result; + wordHasVtSeqs = true; + i += len - 1; + continue; + } + + string delimiter = null; + if (s[i] == ' ' || s[i] == '\t' || s[i] == s_softHyphen) + { + // Soft hyphen = \u00AD - Should break, and add a hyphen if needed. + // If not needed for a break, hyphen should be absent. + delimiter = new string(s[i], 1); } - // Non-breaking space = \u00A0 - ideally shouldn't wrap - // Hard hyphen = \u2011 - Should not break else if (s[i] == s_hardHyphen || s[i] == s_nonBreakingSpace) { - if (valueStrDec.IsDecorated && !sb.EndsWith(PSStyle.Instance.Reset)) + // Non-breaking space = \u00A0 - ideally shouldn't wrap. + // Hard hyphen = \u2011 - Should not break. + delimiter = string.Empty; + } + + if (delimiter is not null) + { + if (wordHasVtSeqs && !sb.EndsWith(PSStyle.Instance.Reset)) { sb.Append(PSStyle.Instance.Reset); } - var result = new GetWordsResult() { Word = sb.ToString(), Delim = string.Empty }; - sb.Clear(); + var result = new GetWordsResult() + { + Word = sb.ToString(), + Delim = delimiter + }; + + sb.Clear().Append(vtSeqs); yield return result; } else @@ -402,6 +419,19 @@ private static IEnumerable GetWords(string s) } } + if (wordHasVtSeqs) + { + if (sb.Length == vtSeqs.Length) + { + // This indicates 'sb' only contains all VT sequences, which may happen when the string ends with a word delimiter. + sb.Clear(); + } + else if (!sb.EndsWith(PSStyle.Instance.Reset)) + { + sb.Append(PSStyle.Instance.Reset); + } + } + yield return new GetWordsResult() { Word = sb.ToString(), Delim = string.Empty }; } @@ -429,9 +459,9 @@ private static StringCollection GenerateLinesWithoutWordWrap(DisplayCells displa } // break string on newlines and process each line separately - string[] lines = SplitLines(val); + List lines = SplitLines(val); - for (int k = 0; k < lines.Length; k++) + for (int k = 0; k < lines.Count; k++) { string currentLine = lines[k]; @@ -547,9 +577,9 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe } // break string on newlines and process each line separately - string[] lines = SplitLines(val); + List lines = SplitLines(val); - for (int k = 0; k < lines.Length; k++) + for (int k = 0; k < lines.Count; k++) { if (lines[k] == null || displayCells.Length(lines[k]) <= firstLineLen) { @@ -562,6 +592,7 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe int lineWidth = firstLineLen; bool firstLine = true; StringBuilder singleLine = new StringBuilder(); + string resetStr = PSStyle.Instance.Reset; foreach (GetWordsResult word in GetWords(lines[k])) { @@ -575,15 +606,15 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe // Add hyphen only if necessary if (wordWidthWithHyphen == spacesLeft) { - wordToAdd += "-"; + wordToAdd = wordToAdd.EndsWith(resetStr) + ? wordToAdd.Insert(wordToAdd.Length - resetStr.Length, "-") + : wordToAdd + "-"; } } - else + else if (!string.IsNullOrEmpty(word.Delim)) { - if (!string.IsNullOrEmpty(word.Delim)) - { - wordToAdd += word.Delim; - } + // Other non-empty delimiters are white-space characters, which we can directly add to the end. + wordToAdd += word.Delim; } int wordWidth = displayCells.Length(wordToAdd); @@ -608,15 +639,37 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe // Word is wider than a single line if (wordWidth > lineWidth) { - foreach (char c in wordToAdd) + Dictionary vtRanges = null; + StringBuilder vtSeqs = null; + if (wordToAdd.Contains(ValueStringDecorated.ESC)) { - char charToAdd = c; - int charWidth = displayCells.Length(c); + vtSeqs = new StringBuilder(); + vtRanges = new Dictionary(); + foreach (Match match in ValueStringDecorated.AnsiRegex.Matches(wordToAdd)) + { + vtRanges.Add(match.Index, match.Length); + } + } - // corner case: we have a two cell character and the current - // display length is one. - // add a single cell arbitrary character instead of the original - // one and keep going + bool hasEscSeqs = false; + for (int i = 0; i < wordToAdd.Length; i++) + { + if (vtRanges?.TryGetValue(i, out int len) == true) + { + var vtSpan = wordToAdd.AsSpan(i, len); + singleLine.Append(vtSpan); + vtSeqs.Append(vtSpan); + + hasEscSeqs = true; + i += len - 1; + continue; + } + + char charToAdd = wordToAdd[i]; + int charWidth = displayCells.Length(charToAdd); + + // Corner case: we have a two cell character and the current display length is one. + // Add a single cell arbitrary character instead of the original one and keep going. if (charWidth > lineWidth) { charToAdd = '?'; @@ -625,9 +678,13 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe if (charWidth > spacesLeft) { + if (hasEscSeqs && !singleLine.EndsWith(resetStr)) + { + singleLine.Append(resetStr); + } + retVal.Add(singleLine.ToString()); - singleLine.Clear(); - singleLine.Append(charToAdd); + singleLine.Clear().Append(vtSeqs).Append(charToAdd); if (firstLine) { @@ -649,8 +706,7 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe if (wordWidth > spacesLeft) { retVal.Add(singleLine.ToString()); - singleLine.Clear(); - singleLine.Append(wordToAdd); + singleLine.Clear().Append(wordToAdd); if (firstLine) { @@ -680,49 +736,78 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe /// /// String to split. /// String array with the values. - internal static string[] SplitLines(string s) + internal static List SplitLines(string s) { - if (string.IsNullOrEmpty(s)) - return new string[1] { s }; + if (string.IsNullOrEmpty(s) || !s.Contains('\n')) + { + return new List(capacity: 1) { s?.Replace("\r", string.Empty) }; + } StringBuilder sb = new StringBuilder(); + List list = new List(); + + StringBuilder vtSeqs = null; + Dictionary vtRanges = null; - foreach (char c in s) + var valueStrDec = new ValueStringDecorated(s); + if (valueStrDec.IsDecorated) { - if (c != '\r') - sb.Append(c); + vtSeqs = new StringBuilder(); + vtRanges = new Dictionary(); + foreach (Match match in ValueStringDecorated.AnsiRegex.Matches(s)) + { + vtRanges.Add(match.Index, match.Length); + } } - return sb.ToString().Split(s_newLineChar); - } - -#if false - internal static string StripNewLines (string s) - { - if (string.IsNullOrEmpty (s)) - return s; - - string[] lines = SplitLines (s); + bool hasVtSeqs = false; + for (int i = 0; i < s.Length; i++) + { + if (vtRanges?.TryGetValue(i, out int len) == true) + { + var vtSpan = s.AsSpan(i, len); + sb.Append(vtSpan); + vtSeqs.Append(vtSpan); - if (lines.Length == 0) - return null; + hasVtSeqs = true; + i += len - 1; + continue; + } - if (lines.Length == 1) - return lines[0]; + char c = s[i]; + if (c == '\n') + { + if (hasVtSeqs && !sb.EndsWith(PSStyle.Instance.Reset)) + { + sb.Append(PSStyle.Instance.Reset); + } - StringBuilder sb = new StringBuilder (); + list.Add(sb.ToString()); + sb.Clear().Append(vtSeqs); + } + else if (c != '\r') + { + sb.Append(c); + } + } - for (int k = 0; k < lines.Length; k++) + if (hasVtSeqs) { - if (k == 0) - sb.Append (lines[k]); - else - sb.Append (" " + lines[k]); + if (sb.Length == vtSeqs.Length) + { + // This indicates 'sb' only contains all VT sequences, which may happen when the string ends with '\n'. + sb.Clear(); + } + else if (!sb.EndsWith(PSStyle.Instance.Reset)) + { + sb.Append(PSStyle.Instance.Reset); + } } - return sb.ToString (); + list.Add(sb.ToString()); + return list; } -#endif + internal static string TruncateAtNewLine(string s) { if (string.IsNullOrEmpty(s)) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs index ddd69ae0ee9..8b14a7af7e2 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System.Globalization; +using System.Collections.Generic; using System.IO; using System.Management.Automation; using System.Management.Automation.Host; @@ -379,10 +380,10 @@ private void WriteLineInternal(string val, int cols) } // check for line breaks - string[] lines = StringManipulationHelper.SplitLines(val); + List lines = StringManipulationHelper.SplitLines(val); // process the substrings as separate lines - for (int k = 0; k < lines.Length; k++) + for (int k = 0; k < lines.Count; k++) { // compute the display length of the string int displayLength = _displayCells.Length(lines[k]); diff --git a/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs b/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs index 16df0c57287..8973d9963e2 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Collections.Specialized; using System.Diagnostics; using System.Management.Automation; @@ -180,12 +181,12 @@ private void WriteProperty(int k, string propertyValue, LineOutput lo) propertyValue = string.Empty; // make sure we honor embedded newlines - string[] lines = StringManipulationHelper.SplitLines(propertyValue); + List lines = StringManipulationHelper.SplitLines(propertyValue); // padding to use in the lines after the first string padding = null; - for (int i = 0; i < lines.Length; i++) + for (int i = 0; i < lines.Count; i++) { string prependString = null; @@ -212,7 +213,7 @@ private void WriteProperty(int k, string propertyValue, LineOutput lo) /// LineOuput to write to. private void WriteSingleLineHelper(string prependString, string line, LineOutput lo) { - if (line == null) + if (line is null) { line = string.Empty; } @@ -223,8 +224,8 @@ private void WriteSingleLineHelper(string prependString, string line, LineOutput // split the lines StringCollection sc = StringManipulationHelper.GenerateLines(lo.DisplayCells, line, fieldCellCount, fieldCellCount); - // padding to use in the lines after the first - string padding = StringUtil.Padding(_propertyLabelsDisplayLength); + // The padding to use in the lines after the first. + string headPadding = null; // display the string collection for (int k = 0; k < sc.Count; k++) @@ -234,17 +235,24 @@ private void WriteSingleLineHelper(string prependString, string line, LineOutput if (k == 0) { - _cachedBuilder - .Append(PSStyle.Instance.Formatting.FormatAccent) - .Append(prependString) - .Append(PSStyle.Instance.Reset) - .Append(str); + if (string.IsNullOrWhiteSpace(prependString)) + { + _cachedBuilder.Append(prependString).Append(str); + } + else + { + _cachedBuilder + .Append(PSStyle.Instance.Formatting.FormatAccent) + .Append(prependString) + .Append(PSStyle.Instance.Reset) + .Append(str); + } } else { - _cachedBuilder - .Append(padding) - .Append(str); + // Lazily calculate the padding to use for the subsequent lines as it's quite often that only the first line exists. + headPadding ??= StringUtil.Padding(_propertyLabelsDisplayLength); + _cachedBuilder.Append(headPadding).Append(str); } if (str.Contains(ValueStringDecorated.ESC) && !str.EndsWith(PSStyle.Instance.Reset)) diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index 24343537e82..2d4b83e06a8 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -342,4 +342,42 @@ Billy Bob… Senior DevOps … 13 $text = Get-Content $outFile -Raw $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") } + + It "Word wrapping for string with escape sequences" { + $expected = @" +`e[32;1mLongDescription : `e[0m`e[33mPowerShell`e[0m + `e[33mscripting`e[0m + `e[33mlanguage`e[0m +"@ + $obj = [pscustomobject] @{ LongDescription = "`e[33mPowerShell scripting language" } + $obj | Format-List | Out-String -Width 35 | Out-File $outFile + + $text = Get-Content $outFile -Raw + $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") + } + + It "Splitting multi-line string with escape sequences" { + $expected = @" +`e[32;1mb : `e[0m`e[33mPowerShell is a task automation and configuration management program from Microsoft,`e[0m + `e[33mconsisting of a command-line shell and the associated scripting language`e[0m +"@ + $obj = [pscustomobject] @{ b = "`e[33mPowerShell is a task automation and configuration management program from Microsoft,`nconsisting of a command-line shell and the associated scripting language" } + $obj | Format-List | Out-File $outFile + + $text = Get-Content $outFile -Raw + $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") + } + + It "Wrapping long word with escape sequences" { + $expected = @" +`e[32;1mb : `e[0m`e[33mC:\repos\PowerShell\src\powershell-w`e[0m + `e[33min-core\bin\Debug\net7.0\win7-x64\pu`e[0m + `e[33mblish\pwsh.exe`e[0m +"@ + $obj = [pscustomobject] @{ b = "`e[33mC:\repos\PowerShell\src\powershell-win-core\bin\Debug\net7.0\win7-x64\publish\pwsh.exe" } + $obj | Format-List | Out-String -Width 40 | Out-File $outFile + + $text = Get-Content $outFile -Raw + $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") + } } From 17c57b10724c6e5c911eb05c2f8bb8f323779347 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 11 May 2022 11:23:44 -0700 Subject: [PATCH 3/7] Add a comment --- .../FormatAndOutput/common/ListWriter.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs b/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs index 8973d9963e2..786a25c2d7b 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs @@ -237,6 +237,8 @@ private void WriteSingleLineHelper(string prependString, string line, LineOutput { if (string.IsNullOrWhiteSpace(prependString)) { + // Sometimes 'prependString' is just padding white spaces. + // We don't need to add formatting escape sequences in such a case. _cachedBuilder.Append(prependString).Append(str); } else From 38bbc036265d67f319384d6a9912956cf7c20173 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 11 May 2022 11:55:57 -0700 Subject: [PATCH 4/7] Address CodeFactor issues --- .../FormatAndOutput/common/ILineOutput.cs | 2 +- stylecop.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs index 8b14a7af7e2..a1f373a5dd5 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs @@ -1,8 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System.Globalization; using System.Collections.Generic; +using System.Globalization; using System.IO; using System.Management.Automation; using System.Management.Automation.Host; diff --git a/stylecop.json b/stylecop.json index 77517f2095b..9653436690c 100644 --- a/stylecop.json +++ b/stylecop.json @@ -25,7 +25,7 @@ }, "namingRules" : { "allowCommonHungarianPrefixes" : true, - "allowedHungarianPrefixes" : [ "n", "r", "l", "i", "io", "is", "fs", "lp", "dw", "h", "rs", "ps", "op", "sb", "my" ] + "allowedHungarianPrefixes" : [ "n", "r", "l", "i", "io", "is", "fs", "lp", "dw", "h", "rs", "ps", "op", "sb", "my", "vt" ] }, "maintainabilityRules" : { "topLevelTypes" : [ From 1ddde6128f3bee64e2b08495ad45a0d39825e76f Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 11 May 2022 12:00:24 -0700 Subject: [PATCH 5/7] Add to another hungarian prefix collection --- Settings.StyleCop | 1 + 1 file changed, 1 insertion(+) diff --git a/Settings.StyleCop b/Settings.StyleCop index 7fa179ee02e..e10c02bdd12 100644 --- a/Settings.StyleCop +++ b/Settings.StyleCop @@ -162,6 +162,7 @@ op my sb + vt From 14c1ae8315c6adad24db2d8cd155d5ca06769300 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 11 May 2022 13:33:15 -0700 Subject: [PATCH 6/7] Fix the test failure --- .../FormatAndOutput/common/ComplexWriter.cs | 17 +++++++++++------ .../engine/Formatting/PSStyle.Tests.ps1 | 4 ++-- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs index f3164ee35d0..a9fa368dc8d 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs @@ -597,24 +597,29 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe foreach (GetWordsResult word in GetWords(lines[k])) { string wordToAdd = word.Word; + string suffix = null; // Handle soft hyphen - if (word.Delim == s_softHyphen.ToString()) + if (word.Delim.Length == 1 && word.Delim[0] == s_softHyphen) { int wordWidthWithHyphen = displayCells.Length(wordToAdd) + displayCells.Length(s_softHyphen); // Add hyphen only if necessary if (wordWidthWithHyphen == spacesLeft) { - wordToAdd = wordToAdd.EndsWith(resetStr) - ? wordToAdd.Insert(wordToAdd.Length - resetStr.Length, "-") - : wordToAdd + "-"; + suffix = "-"; } } else if (!string.IsNullOrEmpty(word.Delim)) { - // Other non-empty delimiters are white-space characters, which we can directly add to the end. - wordToAdd += word.Delim; + suffix = word.Delim; + } + + if (suffix is not null) + { + wordToAdd = wordToAdd.EndsWith(resetStr) + ? wordToAdd.Insert(wordToAdd.Length - resetStr.Length, suffix) + : wordToAdd + suffix; } int wordWidth = displayCells.Length(wordToAdd); diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index 2d4b83e06a8..aa54a63dc05 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -345,8 +345,8 @@ Billy Bob… Senior DevOps … 13 It "Word wrapping for string with escape sequences" { $expected = @" -`e[32;1mLongDescription : `e[0m`e[33mPowerShell`e[0m - `e[33mscripting`e[0m +`e[32;1mLongDescription : `e[0m`e[33mPowerShell `e[0m + `e[33mscripting `e[0m `e[33mlanguage`e[0m "@ $obj = [pscustomobject] @{ LongDescription = "`e[33mPowerShell scripting language" } From e171acd45d63ff0ad60002358004e8901fee1e4b Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 12 May 2022 10:30:30 -0700 Subject: [PATCH 7/7] Address Ilya's feedback --- .../FormatAndOutput/common/ComplexWriter.cs | 28 ++++++++---------- .../FormatAndOutput/common/StringDecorated.cs | 29 ++++++++++++++++++- .../utils/StringUtil.cs | 12 ++------ 3 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs index a9fa368dc8d..0982a8e198e 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs @@ -362,11 +362,7 @@ private static IEnumerable GetWords(string s) if (valueStrDec.IsDecorated) { vtSeqs = new StringBuilder(); - vtRanges = new Dictionary(); - foreach (Match match in ValueStringDecorated.AnsiRegex.Matches(s)) - { - vtRanges.Add(match.Index, match.Length); - } + vtRanges = valueStrDec.EscapeSequenceRanges; } bool wordHasVtSeqs = false; @@ -424,6 +420,9 @@ private static IEnumerable GetWords(string s) if (sb.Length == vtSeqs.Length) { // This indicates 'sb' only contains all VT sequences, which may happen when the string ends with a word delimiter. + // For a word that contains VT sequence only, it's the same as an empty string to the formatting system, + // because nothing will actually be rendered. + // So, we use an empty string in this case to avoid unneeded string allocations. sb.Clear(); } else if (!sb.EndsWith(PSStyle.Instance.Reset)) @@ -646,14 +645,12 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe { Dictionary vtRanges = null; StringBuilder vtSeqs = null; - if (wordToAdd.Contains(ValueStringDecorated.ESC)) + + var valueStrDec = new ValueStringDecorated(wordToAdd); + if (valueStrDec.IsDecorated) { vtSeqs = new StringBuilder(); - vtRanges = new Dictionary(); - foreach (Match match in ValueStringDecorated.AnsiRegex.Matches(wordToAdd)) - { - vtRanges.Add(match.Index, match.Length); - } + vtRanges = valueStrDec.EscapeSequenceRanges; } bool hasEscSeqs = false; @@ -758,11 +755,7 @@ internal static List SplitLines(string s) if (valueStrDec.IsDecorated) { vtSeqs = new StringBuilder(); - vtRanges = new Dictionary(); - foreach (Match match in ValueStringDecorated.AnsiRegex.Matches(s)) - { - vtRanges.Add(match.Index, match.Length); - } + vtRanges = valueStrDec.EscapeSequenceRanges; } bool hasVtSeqs = false; @@ -801,6 +794,9 @@ internal static List SplitLines(string s) if (sb.Length == vtSeqs.Length) { // This indicates 'sb' only contains all VT sequences, which may happen when the string ends with '\n'. + // For a sub-string that contains VT sequence only, it's the same as an empty string to the formatting + // system, because nothing will actually be rendered. + // So, we use an empty string in this case to avoid unneeded string allocations. sb.Clear(); } else if (!sb.EndsWith(PSStyle.Instance.Reset)) diff --git a/src/System.Management.Automation/FormatAndOutput/common/StringDecorated.cs b/src/System.Management.Automation/FormatAndOutput/common/StringDecorated.cs index 9b82cf3d7a7..c2497bdff59 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/StringDecorated.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/StringDecorated.cs @@ -3,6 +3,7 @@ #nullable enable +using System.Collections.Generic; using System.Text.RegularExpressions; namespace System.Management.Automation.Internal @@ -87,6 +88,7 @@ internal struct ValueStringDecorated private readonly bool _isDecorated; private readonly string _text; private string? _plaintextcontent; + private Dictionary? _vtRanges; private string PlainText { @@ -110,6 +112,30 @@ private string PlainText // replace regex with .NET 6 API once available internal static readonly Regex AnsiRegex = new Regex($"{GraphicsRegex}|{CsiRegex}", RegexOptions.Compiled); + /// + /// Get the ranges of all escape sequences in the text. + /// + /// + /// A dictionary with the key being the starting index of an escape sequence, + /// and the value being the length of the escape sequence. + /// + internal Dictionary? EscapeSequenceRanges + { + get + { + if (_isDecorated && _vtRanges is null) + { + _vtRanges = new Dictionary(); + foreach (Match match in AnsiRegex.Matches(_text)) + { + _vtRanges.Add(match.Index, match.Length); + } + } + + return _vtRanges; + } + } + /// /// Initializes a new instance of the struct. /// @@ -118,7 +144,8 @@ public ValueStringDecorated(string text) { _text = text; _isDecorated = text.Contains(ESC); - _plaintextcontent = null; + _plaintextcontent = _isDecorated ? null : text; + _vtRanges = null; } /// diff --git a/src/System.Management.Automation/utils/StringUtil.cs b/src/System.Management.Automation/utils/StringUtil.cs index ed08f425eac..6bdedd03a3b 100644 --- a/src/System.Management.Automation/utils/StringUtil.cs +++ b/src/System.Management.Automation/utils/StringUtil.cs @@ -157,22 +157,16 @@ internal static string VtSubstring(this string str, int startOffset, int length, if (valueStrDec.IsDecorated) { // Handle strings with VT sequences. - var sb = new StringBuilder(capacity: str.Length); bool copyStarted = startOffset == 0; bool hasEscSeqs = false; bool firstNonEscChar = true; - - // Find all escape sequences in the string, and keep track of their starting indexes and length. - var ansiRanges = new Dictionary(); - foreach (Match match in ValueStringDecorated.AnsiRegex.Matches(str)) - { - ansiRanges.Add(match.Index, match.Length); - } + StringBuilder sb = new(capacity: str.Length); + Dictionary vtRanges = valueStrDec.EscapeSequenceRanges; for (int i = 0, offset = 0; i < str.Length; i++) { // Keep all leading ANSI escape sequences. - if (ansiRanges.TryGetValue(i, out int len)) + if (vtRanges.TryGetValue(i, out int len)) { hasEscSeqs = true; sb.Append(str.AsSpan(i, len));