From 7cd2f9522c7612801439455d3198ca2418723a18 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 3 May 2022 18:09:04 -0700 Subject: [PATCH 1/6] Fix formatting truncation to handle strings with VT sequences --- .../FormatAndOutput/common/ComplexWriter.cs | 8 +- .../FormatAndOutput/common/ILineOutput.cs | 122 ++++++++---- .../FormatAndOutput/common/ListWriter.cs | 7 +- .../FormatAndOutput/common/TableWriter.cs | 50 ++--- .../common/Utilities/MshObjectUtil.cs | 1 + .../out-console/ConsoleLineOutput.cs | 148 +++------------ .../FormatAndOutput/out-console/OutConsole.cs | 6 +- .../utils/StringUtil.cs | 176 ++++++++++++++++++ .../engine/Formatting/PSStyle.Tests.ps1 | 99 ++++++++++ 9 files changed, 420 insertions(+), 197 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs index 187775d3072..3adb5a799c2 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ComplexWriter.cs @@ -451,7 +451,7 @@ private static StringCollection GenerateLinesWithoutWordWrap(DisplayCells displa { // we are not at the end of the string, select a sub string // that would fit in the remaining display length - int charactersToAdd = displayCells.GetHeadSplitLength(currentLine, offset, currentDisplayLen); + int charactersToAdd = displayCells.TruncateTail(currentLine, offset, currentDisplayLen); if (charactersToAdd <= 0) { @@ -465,7 +465,7 @@ private static StringCollection GenerateLinesWithoutWordWrap(DisplayCells displa else { // of the given length, add it to the accumulator - accumulator.AddLine(currentLine.Substring(offset, charactersToAdd)); + accumulator.AddLine(currentLine.VtSubstring(offset, charactersToAdd)); } // increase the offset by the # of characters added @@ -474,7 +474,7 @@ private static StringCollection GenerateLinesWithoutWordWrap(DisplayCells displa else { // we reached the last (partial) line, we add it all - accumulator.AddLine(currentLine.Substring(offset)); + accumulator.AddLine(currentLine.VtSubstring(offset)); break; } } @@ -553,7 +553,7 @@ private static StringCollection GenerateLinesWithWordWrap(DisplayCells displayCe // Handle soft hyphen if (word.Delim == s_softHyphen.ToString()) { - int wordWidthWithHyphen = displayCells.Length(wordToAdd) + displayCells.Length(s_softHyphen.ToString()); + int wordWidthWithHyphen = displayCells.Length(wordToAdd) + displayCells.Length(s_softHyphen); // Add hyphen only if necessary if (wordWidthWithHyphen == spacesLeft) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs index 8c672b62063..528232d2c02 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs @@ -15,59 +15,109 @@ namespace Microsoft.PowerShell.Commands.Internal.Format /// /// Base class providing support for string manipulation. /// This class is a tear off class provided by the LineOutput class - /// - /// Assumptions (in addition to the assumptions made for LineOutput): - /// - characters map to one or more character cells - /// - /// NOTE: we provide a base class that is valid for devices that have a - /// 1:1 mapping between a UNICODE character and a display cell. /// internal class DisplayCells { - internal virtual int Length(string str) + /// + /// Calculate the buffer cell length of the given string. + /// + /// String that may contain VT escape sequences. + /// Number of buffer cells the string needs to take. + internal int Length(string str) { return Length(str, 0); } + /// + /// Calculate the buffer cell length of the given string. + /// + /// String that may contain VT escape sequences. + /// + /// When the string doesn't contain VT sequences, it's the starting index. + /// When the string contains VT sequences, it means starting from the 'n-th' char that doesn't belong to a escape sequence. + /// Number of buffer cells the string needs to take. internal virtual int Length(string str, int offset) { - int length = 0; + if (string.IsNullOrEmpty(str)) + { + return 0; + } - foreach (char c in str) + var valueStrDec = new ValueStringDecorated(str); + if (valueStrDec.IsDecorated) { - length += LengthInBufferCells(c); + str = valueStrDec.ToString(OutputRendering.PlainText); } - return length - offset; - } + int length = 0; + for (; offset < str.Length; offset++) + { + length += CharLengthInBufferCells(str[offset]); + } - internal virtual int Length(char character) { return 1; } + return length; + } - internal virtual int GetHeadSplitLength(string str, int displayCells) + /// + /// Calculate the buffer cell length of the given character. + /// + internal virtual int Length(char character) { - return GetHeadSplitLength(str, 0, displayCells); + return CharLengthInBufferCells(character); } - internal virtual int GetHeadSplitLength(string str, int offset, int displayCells) + /// + /// Truncate from the tail of the string. + /// + /// String that may contain VT escape sequences. + /// Number of buffer cells to fit in. + /// + internal int TruncateTail(string str, int displayCells) { - int len = str.Length - offset; - return (len < displayCells) ? len : displayCells; + return TruncateTail(str, offset: 0, displayCells); } - internal virtual int GetTailSplitLength(string str, int displayCells) + /// + /// Truncate from the tail of the string. + /// + /// String that may contain VT escape sequences. + /// + /// When the string doesn't contain VT sequences, it's the starting index. + /// When the string contains VT sequences, it means starting from the 'n-th' char that doesn't belong to a escape sequence. + /// Number of buffer cells to fit in. + /// Number of non-escape-sequence characters from head of the string that can fit in the space. + internal int TruncateTail(string str, int offset, int displayCells) { - return GetTailSplitLength(str, 0, displayCells); + var valueStrDec = new ValueStringDecorated(str); + if (valueStrDec.IsDecorated) + { + str = valueStrDec.ToString(OutputRendering.PlainText); + } + + return GetFitLength(str, offset, displayCells, startFromHead: true); } - internal virtual int GetTailSplitLength(string str, int offset, int displayCells) + /// + /// Truncate from the head of the string. + /// + /// String that may contain VT escape sequences. + /// Number of buffer cells to fit in. + /// Number of non-escape-sequence characters from head of the string that should be skipped. + internal int TruncateHead(string str, int displayCells) { - int len = str.Length - offset; - return (len < displayCells) ? len : displayCells; + var valueStrDec = new ValueStringDecorated(str); + if (valueStrDec.IsDecorated) + { + str = valueStrDec.ToString(OutputRendering.PlainText); + } + + int tailCount = GetFitLength(str, offset: 0, displayCells, startFromHead: false); + return str.Length - tailCount; } #region Helpers - protected static int LengthInBufferCells(char c) + protected static int CharLengthInBufferCells(char c) { // The following is based on http://www.cl.cam.ac.uk/~mgk25/c/wcwidth.c // which is derived from https://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt @@ -94,25 +144,26 @@ protected static int LengthInBufferCells(char c) /// Given a string and a number of display cells, it computes how many /// characters would fit starting from the beginning or end of the string. /// - /// String to be displayed. + /// String to be displayed, which doesn't contain any VT sequences. /// Offset inside the string. /// Number of display cells. - /// If true compute from the head (i.e. k++) else from the tail (i.e. k--). + /// If true compute from the head (i.e. k++) else from the tail (i.e. k--). /// Number of characters that would fit. - protected int GetSplitLengthInternalHelper(string str, int offset, int displayCells, bool head) + protected int GetFitLength(string str, int offset, int displayCells, bool startFromHead) { int filledDisplayCellsCount = 0; // number of cells that are filled in int charactersAdded = 0; // number of characters that fit int currCharDisplayLen; // scratch variable - int k = (head) ? offset : str.Length - 1; - int kFinal = (head) ? str.Length - 1 : offset; + int k = startFromHead ? offset : str.Length - 1; + int kFinal = startFromHead ? str.Length - 1 : offset; while (true) { - if ((head && (k > kFinal)) || ((!head) && (k < kFinal))) + if ((startFromHead && (k > kFinal)) || ((!startFromHead) && (k < kFinal))) { break; } + // compute the cell number for the current character currCharDisplayLen = this.Length(str[k]); @@ -121,6 +172,7 @@ protected int GetSplitLengthInternalHelper(string str, int offset, int displayCe // if we added this character it would not fit, we cannot continue break; } + // keep adding, we fit filledDisplayCellsCount += currCharDisplayLen; charactersAdded++; @@ -132,13 +184,13 @@ protected int GetSplitLengthInternalHelper(string str, int offset, int displayCe break; } - k = (head) ? (k + 1) : (k - 1); + k = startFromHead ? (k + 1) : (k - 1); } return charactersAdded; } - #endregion + #endregion } /// @@ -354,11 +406,11 @@ private void WriteLineInternal(string val, int cols) { // the string is still too long to fit, write the first cols characters // and go back for more wraparound - int splitLen = _displayCells.GetHeadSplitLength(s, cols); - WriteLineInternal(s.Substring(0, splitLen), cols); + int headCount = _displayCells.TruncateTail(s, cols); + WriteLineInternal(s.VtSubstring(0, headCount), cols); // chop off the first fieldWidth characters, already printed - s = s.Substring(splitLen); + s = s.VtSubstring(headCount); if (_displayCells.Length(s) <= cols) { // if we fit, print the tail of the string and we are done diff --git a/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs b/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs index b5c97c3d85d..6031eacd216 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ListWriter.cs @@ -88,19 +88,20 @@ internal void Initialize(string[] propertyNames, int screenColumnWidth, DisplayC for (int k = 0; k < propertyNames.Length; k++) { + string propertyName = propertyNames[k]; if (propertyNameCellCounts[k] < _propertyLabelsDisplayLength) { // shorter than the max, add padding - _propertyLabels[k] = propertyNames[k] + StringUtil.Padding(_propertyLabelsDisplayLength - propertyNameCellCounts[k]); + _propertyLabels[k] = propertyName + StringUtil.Padding(_propertyLabelsDisplayLength - propertyNameCellCounts[k]); } else if (propertyNameCellCounts[k] > _propertyLabelsDisplayLength) { // longer than the max, clip - _propertyLabels[k] = propertyNames[k].Substring(0, dc.GetHeadSplitLength(propertyNames[k], _propertyLabelsDisplayLength)); + _propertyLabels[k] = propertyName.VtSubstring(0, dc.TruncateTail(propertyName, _propertyLabelsDisplayLength)); } else { - _propertyLabels[k] = propertyNames[k]; + _propertyLabels[k] = propertyName; } _propertyLabels[k] += Separator; diff --git a/src/System.Management.Automation/FormatAndOutput/common/TableWriter.cs b/src/System.Management.Automation/FormatAndOutput/common/TableWriter.cs index fb5ef0bf930..eae65766bb9 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/TableWriter.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/TableWriter.cs @@ -42,8 +42,6 @@ private sealed class ScreenInfo private ScreenInfo _si; - private const char ESC = '\u001b'; - private List _header; internal static int ComputeWideViewBestItemsPerRowFit(int stringLen, int screenColumns) @@ -457,8 +455,10 @@ private string GenerateRow(string[] values, ReadOnlySpan alignment, Display } } - sb.Append(GenerateRowField(values[k], _si.columnInfo[k].width, alignment[k], dc, addPadding)); - if (values[k] is not null && values[k].Contains(ESC)) + string rowField = GenerateRowField(values[k], _si.columnInfo[k].width, alignment[k], dc, addPadding); + sb.Append(rowField); + + if (rowField is not null && rowField.Contains(ValueStringDecorated.ESC) && !rowField.AsSpan().TrimEnd().EndsWith(PSStyle.Instance.Reset)) { // Reset the console output if the content of this column contains ESC sb.Append(PSStyle.Instance.Reset); @@ -472,9 +472,7 @@ private static string GenerateRowField(string val, int width, int alignment, Dis { // make sure the string does not have any embedded in it string s = StringManipulationHelper.TruncateAtNewLine(val); - - string currentValue = s; - int currentValueDisplayLength = dc.Length(currentValue); + int currentValueDisplayLength = dc.Length(s); if (currentValueDisplayLength < width) { @@ -531,18 +529,10 @@ private static string GenerateRowField(string val, int width, int alignment, Dis case TextAlignment.Right: { // get from "abcdef" to "...f" - int tailCount = dc.GetTailSplitLength(s, truncationDisplayLength); - s = s.Substring(s.Length - tailCount); - s = PSObjectHelper.Ellipsis + s; - } - - break; - - case TextAlignment.Center: - { - // get from "abcdef" to "a..." - s = s.Substring(0, dc.GetHeadSplitLength(s, truncationDisplayLength)); - s += PSObjectHelper.Ellipsis; + s = s.VtSubstring( + startOffset: dc.TruncateHead(s, truncationDisplayLength), + prependStr: PSObjectHelper.EllipsisStr, + appendStr: null); } break; @@ -551,8 +541,11 @@ private static string GenerateRowField(string val, int width, int alignment, Dis { // left align is the default // get from "abcdef" to "a..." - s = s.Substring(0, dc.GetHeadSplitLength(s, truncationDisplayLength)); - s += PSObjectHelper.Ellipsis; + s = s.VtSubstring( + startOffset: 0, + length: dc.TruncateTail(s, truncationDisplayLength), + prependStr: null, + appendStr: PSObjectHelper.EllipsisStr); } break; @@ -561,23 +554,12 @@ private static string GenerateRowField(string val, int width, int alignment, Dis else { // not enough space for the ellipsis, just truncate at the width - int len = width; - switch (alignment) { case TextAlignment.Right: { // get from "abcdef" to "f" - int tailCount = dc.GetTailSplitLength(s, len); - s = s.Substring(s.Length - tailCount, tailCount); - } - - break; - - case TextAlignment.Center: - { - // get from "abcdef" to "a" - s = s.Substring(0, dc.GetHeadSplitLength(s, len)); + s = s.VtSubstring(startOffset: dc.TruncateHead(s, width)); } break; @@ -586,7 +568,7 @@ private static string GenerateRowField(string val, int width, int alignment, Dis { // left align is the default // get from "abcdef" to "a" - s = s.Substring(0, dc.GetHeadSplitLength(s, len)); + s = s.VtSubstring(startOffset: 0, length: dc.TruncateTail(s, width)); } break; diff --git a/src/System.Management.Automation/FormatAndOutput/common/Utilities/MshObjectUtil.cs b/src/System.Management.Automation/FormatAndOutput/common/Utilities/MshObjectUtil.cs index 7760e86b13d..8d95f8fd9a1 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/Utilities/MshObjectUtil.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/Utilities/MshObjectUtil.cs @@ -26,6 +26,7 @@ internal static class PSObjectHelper #endregion tracer internal const char Ellipsis = '\u2026'; + internal const string EllipsisStr = "\u2026"; internal static string PSObjectIsOfExactType(Collection typeNames) { diff --git a/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs b/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs index f3ed3ba6189..1275ba9695f 100644 --- a/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs +++ b/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs @@ -1,11 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -// NOTE: define this if you want to test the output on US machine and ASCII -// characters -//#define TEST_MULTICELL_ON_SINGLE_CELL_LOCALE - using System; +using System.Collections.Generic; using System.Collections.Specialized; using System.Management.Automation; using System.Management.Automation.Internal; @@ -17,70 +14,12 @@ namespace Microsoft.PowerShell.Commands.Internal.Format { -#if TEST_MULTICELL_ON_SINGLE_CELL_LOCALE - - /// - /// Test class to provide easily overridable behavior for testing on US machines - /// using US data. - /// NOTE: the class just forces any uppercase letter [A-Z] to be prepended - /// with an underscore (e.g. "A" becomes "_A", but "a" stays the same) - /// - internal class DisplayCellsTest : DisplayCells - { - internal override int Length(string str, int offset) - { - int len = 0; - for (int k = offset; k < str.Length; k++) - { - len += this.Length(str[k]); - } - - return len; - } - - internal override int Length(char character) - { - if (character >= 'A' && character <= 'Z') - return 2; - return 1; - } - - internal override int GetHeadSplitLength(string str, int offset, int displayCells) - { - return GetSplitLengthInternalHelper(str, offset, displayCells, true); - } - - internal override int GetTailSplitLength(string str, int offset, int displayCells) - { - return GetSplitLengthInternalHelper(str, offset, displayCells, false); - } - - internal string GenerateTestString(string str) - { - StringBuilder sb = new StringBuilder(); - for (int k = 0; k < str.Length; k++) - { - char ch = str[k]; - if (this.Length(ch) == 2) - { - sb.Append('_'); - } - - sb.Append(ch); - } - - return sb.ToString(); - } - - } -#endif - /// /// Tear off class. /// - internal class DisplayCellsPSHost : DisplayCells + internal class DisplayCellsHost : DisplayCells { - internal DisplayCellsPSHost(PSHostRawUserInterface rawUserInterface) + internal DisplayCellsHost(PSHostRawUserInterface rawUserInterface) { _rawUserInterface = rawUserInterface; } @@ -99,30 +38,26 @@ internal override int Length(string str, int offset) try { - return _rawUserInterface.LengthInBufferCells(str, offset); - } - catch - { - // thrown when external host rawui is not implemented, in which case - // we will fallback to the default value. - } + var valueStrDec = new ValueStringDecorated(str); + if (valueStrDec.IsDecorated) + { + str = valueStrDec.ToString(OutputRendering.PlainText); + } - return str.Length - offset; - } + int length = 0; + for (; offset < str.Length; offset++) + { + length += _rawUserInterface.LengthInBufferCells(str[offset]); + } - internal override int Length(string str) - { - try - { - return _rawUserInterface.LengthInBufferCells(str); + return length; } catch { // thrown when external host rawui is not implemented, in which case // we will fallback to the default value. + return base.Length(str, offset); } - - return string.IsNullOrEmpty(str) ? 0 : str.Length; } internal override int Length(char character) @@ -135,19 +70,8 @@ internal override int Length(char character) { // thrown when external host rawui is not implemented, in which case // we will fallback to the default value. + return base.Length(character); } - - return 1; - } - - internal override int GetHeadSplitLength(string str, int offset, int displayCells) - { - return GetSplitLengthInternalHelper(str, offset, displayCells, true); - } - - internal override int GetTailSplitLength(string str, int offset, int displayCells) - { - return GetSplitLengthInternalHelper(str, offset, displayCells, false); } private readonly PSHostRawUserInterface _rawUserInterface; @@ -163,6 +87,8 @@ internal sealed class ConsoleLineOutput : LineOutput internal static readonly PSTraceSource tracer = PSTraceSource.GetTracer("ConsoleLineOutput", "ConsoleLineOutput"); #endregion tracer + private static readonly HashSet s_psHost = new(StringComparer.Ordinal) { "ConsoleHost", "Visual Studio Code Host" }; + #region LineOutput implementation /// /// The # of columns is just the width of the screen buffer (not the @@ -241,12 +167,13 @@ internal override DisplayCells DisplayCells get { CheckStopProcessing(); - if (_displayCellsPSHost != null) + if (_displayCellsHost != null) { - return _displayCellsPSHost; + return _displayCellsHost; } + // fall back if we do not have a Msh host specific instance - return _displayCellsPSHost; + return _displayCellsDefault; } } #endregion @@ -254,17 +181,17 @@ internal override DisplayCells DisplayCells /// /// Constructor for the ConsoleLineOutput. /// - /// PSHostUserInterface to wrap. + /// PSHostUserInterface to wrap. /// True if we require prompting for page breaks. /// Error context to throw exceptions. - internal ConsoleLineOutput(PSHostUserInterface hostConsole, bool paging, TerminatingErrorContext errorContext) + internal ConsoleLineOutput(PSHost host, bool paging, TerminatingErrorContext errorContext) { - if (hostConsole == null) - throw PSTraceSource.NewArgumentNullException(nameof(hostConsole)); + if (host == null) + throw PSTraceSource.NewArgumentNullException(nameof(host)); if (errorContext == null) throw PSTraceSource.NewArgumentNullException(nameof(errorContext)); - _console = hostConsole; + _console = host.UI; _errorContext = errorContext; if (paging) @@ -276,17 +203,11 @@ internal ConsoleLineOutput(PSHostUserInterface hostConsole, bool paging, Termina _prompt = new PromptHandler(promptString, this); } - PSHostRawUserInterface raw = _console.RawUI; - if (raw != null) + if (!s_psHost.Contains(host.Name) && _console.RawUI is not null) { - tracer.WriteLine("there is a valid raw interface"); -#if TEST_MULTICELL_ON_SINGLE_CELL_LOCALE - // create a test instance with fake behavior - this._displayCellsPSHost = new DisplayCellsTest(); -#else // set only if we have a valid raw interface - _displayCellsPSHost = new DisplayCellsPSHost(raw); -#endif + tracer.WriteLine("there is a valid raw interface"); + _displayCellsHost = new DisplayCellsHost(_console.RawUI); } // instantiate the helper to do the line processing when ILineOutput.WriteXXX() is called @@ -309,9 +230,6 @@ internal ConsoleLineOutput(PSHostUserInterface hostConsole, bool paging, Termina /// String to write. private void OnWriteLine(string s) { -#if TEST_MULTICELL_ON_SINGLE_CELL_LOCALE - s = ((DisplayCellsTest)this._displayCellsPSHost).GenerateTestString(s); -#endif // Do any default transcription. _console.TranscribeResult(s); @@ -356,10 +274,6 @@ private void OnWriteLine(string s) /// String to write. private void OnWrite(string s) { -#if TEST_MULTICELL_ON_SINGLE_CELL_LOCALE - s = ((DisplayCellsTest)this._displayCellsPSHost).GenerateTestString(s); -#endif - switch (this.WriteStream) { case WriteStreamType.Error: @@ -615,7 +529,7 @@ internal PromptResponse PromptUser(PSHostUserInterface console) /// /// Msh host specific string manipulation helper. /// - private readonly DisplayCells _displayCellsPSHost; + private readonly DisplayCells _displayCellsHost; /// /// Reference to error context to throw Msh exceptions. diff --git a/src/System.Management.Automation/FormatAndOutput/out-console/OutConsole.cs b/src/System.Management.Automation/FormatAndOutput/out-console/OutConsole.cs index 2582a1e68db..8d51f42de8c 100644 --- a/src/System.Management.Automation/FormatAndOutput/out-console/OutConsole.cs +++ b/src/System.Management.Automation/FormatAndOutput/out-console/OutConsole.cs @@ -65,8 +65,7 @@ public OutDefaultCommand() /// protected override void BeginProcessing() { - PSHostUserInterface console = this.Host.UI; - ConsoleLineOutput lineOutput = new ConsoleLineOutput(console, false, new TerminatingErrorContext(this)); + var lineOutput = new ConsoleLineOutput(Host, false, new TerminatingErrorContext(this)); ((OutputManagerInner)this.implementation).LineOutput = lineOutput; @@ -206,8 +205,7 @@ public SwitchParameter Paging /// protected override void BeginProcessing() { - PSHostUserInterface console = this.Host.UI; - ConsoleLineOutput lineOutput = new ConsoleLineOutput(console, _paging, new TerminatingErrorContext(this)); + var lineOutput = new ConsoleLineOutput(Host, _paging, new TerminatingErrorContext(this)); ((OutputManagerInner)this.implementation).LineOutput = lineOutput; base.BeginProcessing(); diff --git a/src/System.Management.Automation/utils/StringUtil.cs b/src/System.Management.Automation/utils/StringUtil.cs index c84b3f5e9fe..87093f5a1f1 100644 --- a/src/System.Management.Automation/utils/StringUtil.cs +++ b/src/System.Management.Automation/utils/StringUtil.cs @@ -1,9 +1,12 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections.Generic; using System.Globalization; using System.Management.Automation.Host; using System.Threading; +using System.Text; +using System.Text.RegularExpressions; using Dbg = System.Management.Automation.Diagnostics; @@ -93,5 +96,178 @@ internal static string DashPadding(int count) return result; } + + /// + /// Substring implementation that takes into account the VT escape sequences. + /// + /// String that may contain VT escape sequences. + /// + /// When the string doesn't contain VT sequences, it's the starting index. + /// When the string contains VT sequences, it means starting from the 'n-th' char that doesn't belong to a escape sequence. + /// + internal static string VtSubstring(this string str, int startOffset) + { + return VtSubstring(str, 0, int.MaxValue, prependStr: null, appendStr: null); + } + + /// + /// Substring implementation that takes into account the VT escape sequences. + /// + /// String that may contain VT escape sequences. + /// + /// When the string doesn't contain VT sequences, it's the starting index. + /// When the string contains VT sequences, it means starting from the 'n-th' char that doesn't belong to a escape sequence. + /// Number of non-escape-sequence characters to be included in the substring. + internal static string VtSubstring(this string str, int startOffset, int length) + { + return VtSubstring(str, startOffset, length, prependStr: null, appendStr: null); + } + + /// + /// Substring implementation that takes into account the VT escape sequences. + /// + /// String that may contain VT escape sequences. + /// + /// When the string doesn't contain VT sequences, it's the starting index. + /// When the string contains VT sequences, it means starting from the 'n-th' char that doesn't belong to a escape sequence. + /// The string to be prepended to the substring. + /// The string to be appended to the substring. + internal static string VtSubstring(this string str, int startOffset, string prependStr, string appendStr) + { + return VtSubstring(str, 0, int.MaxValue, prependStr, appendStr); + } + + /// + /// Substring implementation that takes into account the VT escape sequences. + /// + /// String that may contain VT escape sequences. + /// + /// When the string doesn't contain VT sequences, it's the starting index. + /// When the string contains VT sequences, it means starting from the 'n-th' char that doesn't belong to a escape sequence. + /// Number of non-escape-sequence characters to be included in the substring. + /// The string to be prepended to the substring. + /// The string to be appended to the substring. + internal static string VtSubstring(this string str, int startOffset, int length, string prependStr, string appendStr) + { + var valueStrDec = new ValueStringDecorated(str); + 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); + } + + for (int i = 0, offset = 0; i < str.Length; i++) + { + // Keep all leading ANSI escape sequences. + if (ansiRanges.TryGetValue(i, out int len)) + { + hasEscSeqs = true; + sb.Append(str.AsSpan(i, len)); + + i += len - 1; + continue; + } + + // OK, now we get a non-escape-sequence character. + if (copyStarted) + { + if (firstNonEscChar) + { + // Prepend the string before we copy the first non-escape-sequence character. + sb.Append(prependStr); + firstNonEscChar = false; + } + + // Copy this character if we've started the copy. + sb.Append(str[i]); + // Increment 'offset' to keep track of number of non-escape-sequence characters we've copied. + offset++; + } + else if (++offset == startOffset) + { + // We've skipped enough non-escape-sequence characters, and will be copying the next one. + copyStarted = true; + // Reset 'offset' and from now on use it to track the number of copied non-escape-sequence characters. + offset = 0; + continue; + } + + // If the number of copied non-escape-sequence characters has reached the specified length, done copying. + if (offset == length) + { + break; + } + } + + if (hasEscSeqs) + { + string resetStr = PSStyle.Instance.Reset; + bool endsWithReset = sb.EndsWith(resetStr); + if (endsWithReset) + { + // Append the given string before the reset VT sequence. + sb.Insert(sb.Length - resetStr.Length, appendStr); + } + else + { + // Append the given string and add the reset VT sequence. + sb.Append(appendStr).Append(resetStr); + } + } + else + { + sb.Append(appendStr); + } + + return sb.ToString(); + } + + // Handle strings without VT sequences. + if (length == int.MaxValue) + { + length = str.Length - startOffset; + } + + if (prependStr is null && appendStr is null) + { + return str.Substring(startOffset, length); + } + else + { + int capacity = length + prependStr?.Length ?? 0 + appendStr?.Length ?? 0; + return new StringBuilder(prependStr, capacity) + .Append(str, startOffset, length) + .Append(appendStr) + .ToString(); + } + } + + internal static bool EndsWith(this StringBuilder sb, string value) + { + if (sb.Length < value.Length) + { + return false; + } + + int offset = sb.Length - value.Length; + for (int i = 0; i < value.Length; i++) + { + if (sb[offset + i] != value[i]) + { + return false; + } + } + + return true; + } } } diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index 04c13c5e04e..927d181f91f 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -238,3 +238,102 @@ Describe 'Tests for $PSStyle automatic variable' { } } } + +Describe 'Handle strings with escape sequences in formatting' { + + BeforeAll { + function Get-DemoObjects { + [PSCustomObject]@{PSTypeName = "User"; Name = "Bob Saggat"; Tenure = 2; Role = "Developer" } + [PSCustomObject]@{PSTypeName = "User"; Name = "John Seymour"; Tenure = 6; Role = "Sw Engineer" } + [PSCustomObject]@{PSTypeName = "User"; Name = "Billy Bob Thorton"; Tenure = 13; Role = "Senior DevOps Engineer" } + } + + $colors = @("`e[32m", "`e[34m", "`e[33m", "`e[31m", "`e[33m", "`e[34m", "`e[32m") + $outFile = "$TestDrive\outFile.txt" + } + + It 'Truncation for strings with no escape sequences' { + $expected = @" +`e[32;1mName Role YIR`e[0m +`e[32;1m---- ---- ---`e[0m +Bob Saggat Developer 2 +John Seym… Sw Engineer 6 +Billy Bob… Senior DevOps … 13 +"@ + Get-DemoObjects | + Format-Table @{Width = 10; Name = "Name"; E = { $_.Name }}, + @{Width = 15; Name = "Role"; E = { $_.Role }}, + @{Width = 3; Name = "YIR"; E = { $_.Tenure }} | + Out-File $outFile + + $text = Get-Content $outFile -Raw + $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") + } + + It "Truncation for strings with escape sequences - TableView-1" { + $expected = @" +`e[32;1mName Role YIR`e[0m +`e[32;1m---- ---- ---`e[0m +`e[32mBob Saggat`e[39m`e[0m Developer 2 +`e[33mJohn Seym…`e[0m Sw Engineer 6 +`e[31mBilly Bob…`e[0m Senior DevOps … 13 +"@ + Get-DemoObjects | + Format-Table @{Width = 10; Name = "Name"; E = { + $index = [array]::BinarySearch(@(3, 5, 8), $_.Tenure) + $color = $colors[$index] + $color + $_.Name + "`e[39m"} + }, + @{Width = 15; Name = "Role"; E = { $_.Role }}, + @{Width = 3; Name = "YIR"; E = { $_.Tenure }} | + Out-File $outFile + + $text = Get-Content $outFile -Raw + $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") + } + + It "Truncation for strings with escape sequences - TableView-2" { + $expected = @" +`e[32;1mName Role YIR`e[0m +`e[32;1m---- ---- ---`e[0m +`e[32mBob Saggat`e[39m`e[0m Developer`e[0m 2 +`e[33mJohn Seym…`e[0m `e[1;33mSw Engineer`e[0m 6 +`e[31mBilly Bob…`e[0m `e[42m`e[1;33mSenior DevOps …`e[0m 13 +"@ + Get-DemoObjects | + Format-Table @{Width = 10; Name = "Name"; E = { + $index = [array]::BinarySearch(@(3, 5, 8), $_.Tenure) + $color = $colors[$index] + $color + $_.Name + "`e[39m"} + }, + @{Width = 15; Name = "Role"; E = { + $color = -join $(switch -regex ($_.Role){ + "Senior" { "`e[42m" } + "Engineer" { "`e[1;33m" } + }) + $color + $_.Role + "`e[0m"}}, + @{Width = 3; Name = "YIR"; E = { $_.Tenure }} | + Out-File $outFile + + $text = Get-Content $outFile -Raw + $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") + } + + It "Truncation for strings with escape sequences - WideView" { + $expected = @" +`e[32mBob Saggat`e[39m `e[0m `e[33mJohn Seymour`e[39m`e[0m +`e[31mBilly Bob Thorton`e[39m `e[0m `e[32mBob Saggat`e[39m`e[0m +`e[33mJohn Seymour`e[39m `e[0m `e[31mBilly Bob Thorton`e[39m`e[0m +"@ + (Get-DemoObjects) + (Get-DemoObjects) | + Format-Wide @{E = { + $index = [array]::BinarySearch(@(3, 5, 8), $_.Tenure) + $color = $colors[$index] + $color + $_.Name + "`e[39m" } + } -Column 2 | + Out-String -Width 47 | Out-File $outFile + + $text = Get-Content $outFile -Raw + $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") + } +} From 70b4aaf42ffd74636220df2c20274f3ac930b294 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 3 May 2022 22:44:58 -0700 Subject: [PATCH 2/6] Fix a minor issue + add diagnosis to test --- src/System.Management.Automation/utils/StringUtil.cs | 4 ++-- test/powershell/engine/Formatting/PSStyle.Tests.ps1 | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/utils/StringUtil.cs b/src/System.Management.Automation/utils/StringUtil.cs index 87093f5a1f1..700b69ca0ef 100644 --- a/src/System.Management.Automation/utils/StringUtil.cs +++ b/src/System.Management.Automation/utils/StringUtil.cs @@ -107,7 +107,7 @@ internal static string DashPadding(int count) /// internal static string VtSubstring(this string str, int startOffset) { - return VtSubstring(str, 0, int.MaxValue, prependStr: null, appendStr: null); + return VtSubstring(str, startOffset, int.MaxValue, prependStr: null, appendStr: null); } /// @@ -134,7 +134,7 @@ internal static string VtSubstring(this string str, int startOffset, int length) /// The string to be appended to the substring. internal static string VtSubstring(this string str, int startOffset, string prependStr, string appendStr) { - return VtSubstring(str, 0, int.MaxValue, prependStr, appendStr); + return VtSubstring(str, startOffset, int.MaxValue, prependStr, appendStr); } /// diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index 927d181f91f..3351a5c4cdf 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -267,6 +267,7 @@ Billy Bob… Senior DevOps … 13 Out-File $outFile $text = Get-Content $outFile -Raw + Write-Verbose -Verbose $text $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") } From aa1a3e50e174e926ce5410fd14237f4b728dfd62 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 4 May 2022 10:41:45 -0700 Subject: [PATCH 3/6] Try to fix the test --- test/powershell/engine/Formatting/PSStyle.Tests.ps1 | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 index 3351a5c4cdf..24343537e82 100644 --- a/test/powershell/engine/Formatting/PSStyle.Tests.ps1 +++ b/test/powershell/engine/Formatting/PSStyle.Tests.ps1 @@ -248,10 +248,16 @@ Describe 'Handle strings with escape sequences in formatting' { [PSCustomObject]@{PSTypeName = "User"; Name = "Billy Bob Thorton"; Tenure = 13; Role = "Senior DevOps Engineer" } } + $oldOutputRendering = $PSStyle.OutputRendering + $PSStyle.OutputRendering = [System.Management.Automation.OutputRendering]::Ansi $colors = @("`e[32m", "`e[34m", "`e[33m", "`e[31m", "`e[33m", "`e[34m", "`e[32m") $outFile = "$TestDrive\outFile.txt" } + AfterAll { + $PSStyle.OutputRendering = $oldOutputRendering + } + It 'Truncation for strings with no escape sequences' { $expected = @" `e[32;1mName Role YIR`e[0m @@ -267,7 +273,6 @@ Billy Bob… Senior DevOps … 13 Out-File $outFile $text = Get-Content $outFile -Raw - Write-Verbose -Verbose $text $text.Trim().Replace("`r", "") | Should -BeExactly $expected.Replace("`r", "") } From daeabf4927d0b2c1d4e83195179ac4035da15756 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 4 May 2022 11:33:10 -0700 Subject: [PATCH 4/6] Update cgmanifest.json and address some CodeFactor issues --- .../FormatAndOutput/common/ILineOutput.cs | 6 ++++-- .../FormatAndOutput/out-console/ConsoleLineOutput.cs | 9 +++++++-- src/System.Management.Automation/utils/StringUtil.cs | 8 +++++++- tools/cgmanifest.json | 2 +- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs index 528232d2c02..475f9a41e33 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs @@ -14,7 +14,7 @@ namespace Microsoft.PowerShell.Commands.Internal.Format { /// /// Base class providing support for string manipulation. - /// This class is a tear off class provided by the LineOutput class + /// This class is a tear off class provided by the LineOutput class. /// internal class DisplayCells { @@ -61,6 +61,8 @@ internal virtual int Length(string str, int offset) /// /// Calculate the buffer cell length of the given character. /// + /// + /// Number of buffer cells the character needs to take. internal virtual int Length(char character) { return CharLengthInBufferCells(character); @@ -71,7 +73,7 @@ internal virtual int Length(char character) /// /// String that may contain VT escape sequences. /// Number of buffer cells to fit in. - /// + /// Number of non-escape-sequence characters from head of the string that can fit in the space. internal int TruncateTail(string str, int displayCells) { return TruncateTail(str, offset: 0, displayCells); diff --git a/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs b/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs index 1275ba9695f..eab71fd9657 100644 --- a/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs +++ b/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs @@ -187,9 +187,14 @@ internal override DisplayCells DisplayCells internal ConsoleLineOutput(PSHost host, bool paging, TerminatingErrorContext errorContext) { if (host == null) + { throw PSTraceSource.NewArgumentNullException(nameof(host)); + } + if (errorContext == null) + { throw PSTraceSource.NewArgumentNullException(nameof(errorContext)); + } _console = host.UI; _errorContext = errorContext; @@ -197,8 +202,8 @@ internal ConsoleLineOutput(PSHost host, bool paging, TerminatingErrorContext err if (paging) { tracer.WriteLine("paging is needed"); - // if we need to do paging, instantiate a prompt handler - // that will take care of the screen interaction + + // If we need to do paging, instantiate a prompt handler that will take care of the screen interaction string promptString = StringUtil.Format(FormatAndOut_out_xxx.ConsoleLineOutput_PagingPrompt); _prompt = new PromptHandler(promptString, this); } diff --git a/src/System.Management.Automation/utils/StringUtil.cs b/src/System.Management.Automation/utils/StringUtil.cs index 700b69ca0ef..ed08f425eac 100644 --- a/src/System.Management.Automation/utils/StringUtil.cs +++ b/src/System.Management.Automation/utils/StringUtil.cs @@ -4,9 +4,9 @@ using System.Collections.Generic; using System.Globalization; using System.Management.Automation.Host; -using System.Threading; using System.Text; using System.Text.RegularExpressions; +using System.Threading; using Dbg = System.Management.Automation.Diagnostics; @@ -105,6 +105,7 @@ internal static string DashPadding(int count) /// When the string doesn't contain VT sequences, it's the starting index. /// When the string contains VT sequences, it means starting from the 'n-th' char that doesn't belong to a escape sequence. /// + /// The requested substring. internal static string VtSubstring(this string str, int startOffset) { return VtSubstring(str, startOffset, int.MaxValue, prependStr: null, appendStr: null); @@ -118,6 +119,7 @@ internal static string VtSubstring(this string str, int startOffset) /// When the string doesn't contain VT sequences, it's the starting index. /// When the string contains VT sequences, it means starting from the 'n-th' char that doesn't belong to a escape sequence. /// Number of non-escape-sequence characters to be included in the substring. + /// The requested substring. internal static string VtSubstring(this string str, int startOffset, int length) { return VtSubstring(str, startOffset, length, prependStr: null, appendStr: null); @@ -132,6 +134,7 @@ internal static string VtSubstring(this string str, int startOffset, int length) /// When the string contains VT sequences, it means starting from the 'n-th' char that doesn't belong to a escape sequence. /// The string to be prepended to the substring. /// The string to be appended to the substring. + /// The requested substring. internal static string VtSubstring(this string str, int startOffset, string prependStr, string appendStr) { return VtSubstring(str, startOffset, int.MaxValue, prependStr, appendStr); @@ -147,6 +150,7 @@ internal static string VtSubstring(this string str, int startOffset, string prep /// Number of non-escape-sequence characters to be included in the substring. /// The string to be prepended to the substring. /// The string to be appended to the substring. + /// The requested substring. internal static string VtSubstring(this string str, int startOffset, int length, string prependStr, string appendStr) { var valueStrDec = new ValueStringDecorated(str); @@ -189,6 +193,7 @@ internal static string VtSubstring(this string str, int startOffset, int length, // Copy this character if we've started the copy. sb.Append(str[i]); + // Increment 'offset' to keep track of number of non-escape-sequence characters we've copied. offset++; } @@ -196,6 +201,7 @@ internal static string VtSubstring(this string str, int startOffset, int length, { // We've skipped enough non-escape-sequence characters, and will be copying the next one. copyStarted = true; + // Reset 'offset' and from now on use it to track the number of copied non-escape-sequence characters. offset = 0; continue; diff --git a/tools/cgmanifest.json b/tools/cgmanifest.json index e3042adc9e9..a8d9a620e16 100644 --- a/tools/cgmanifest.json +++ b/tools/cgmanifest.json @@ -695,7 +695,7 @@ "Type": "nuget", "Nuget": { "Name": "StyleCop.Analyzers.Unstable", - "Version": "1.2.0.406" + "Version": "1.2.0.435" } }, "DevelopmentDependency": true From dbe42a5671c5e23f1744124269c316b4267a81ea Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 4 May 2022 11:39:53 -0700 Subject: [PATCH 5/6] Minor fix --- .../FormatAndOutput/common/ILineOutput.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs index 475f9a41e33..ddd69ae0ee9 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/ILineOutput.cs @@ -161,7 +161,7 @@ protected int GetFitLength(string str, int offset, int displayCells, bool startF int kFinal = startFromHead ? str.Length - 1 : offset; while (true) { - if ((startFromHead && (k > kFinal)) || ((!startFromHead) && (k < kFinal))) + if ((startFromHead && k > kFinal) || (!startFromHead && k < kFinal)) { break; } From d1824b287b40c8a664b90d5c724e69c749d2cef2 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 4 May 2022 11:47:41 -0700 Subject: [PATCH 6/6] Add a comment --- .../FormatAndOutput/out-console/ConsoleLineOutput.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs b/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs index eab71fd9657..e8379d3f321 100644 --- a/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs +++ b/src/System.Management.Automation/FormatAndOutput/out-console/ConsoleLineOutput.cs @@ -87,6 +87,9 @@ internal sealed class ConsoleLineOutput : LineOutput internal static readonly PSTraceSource tracer = PSTraceSource.GetTracer("ConsoleLineOutput", "ConsoleLineOutput"); #endregion tracer + /// + /// The default buffer cell calculation already works for the PowerShell console host and Visual studio code host. + /// private static readonly HashSet s_psHost = new(StringComparer.Ordinal) { "ConsoleHost", "Visual Studio Code Host" }; #region LineOutput implementation