From 0b1f3af6f2d580f4c551379c79be6e44bf5152df Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 23 Oct 2018 16:17:22 -0700 Subject: [PATCH 1/5] Replace bad StringReader usage with String.Split() --- .../Workspace/ScriptFile.cs | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs index e8cb7aac1..0cab797e6 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs @@ -20,6 +20,12 @@ public class ScriptFile { #region Private Fields + private static readonly string[] s_newlines = new [] + { + "\n", + "\r\n" + }; + private Token[] scriptTokens; private Version powerShellVersion; @@ -215,26 +221,7 @@ internal static List GetLinesInternal(string text) throw new ArgumentNullException(nameof(text)); } - // ReadLine returns null immediately for empty string, so special case it. - if (text.Length == 0) - { - return new List {string.Empty}; - } - - using (var reader = new StringReader(text)) - { - // 50 is a rough guess for typical average line length, this saves some list - // resizes in the common case and does not hurt meaningfully if we're wrong. - var list = new List(text.Length / 50); - string line; - - while ((line = reader.ReadLine()) != null) - { - list.Add(line); - } - - return list; - } + return new List(text.Split(s_newlines, StringSplitOptions.None)); } /// From f002a6f4e60299cda8e664687e34dbaf7ee880ed Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 24 Oct 2018 10:34:59 -0700 Subject: [PATCH 2/5] Change some whitespace --- src/PowerShellEditorServices/Workspace/ScriptFile.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs index 0cab797e6..80215eeca 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs @@ -173,8 +173,8 @@ public ScriptFile( new StringReader(initialBuffer), powerShellVersion) { - } + /// /// Creates a new ScriptFile instance with the specified filepath. /// @@ -191,7 +191,6 @@ public ScriptFile( File.ReadAllText(filePath), powerShellVersion) { - } #endregion From 705eff56788048793acf5bee901fade653847365 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 24 Oct 2018 10:35:38 -0700 Subject: [PATCH 3/5] Reorder string splitting --- src/PowerShellEditorServices/Workspace/ScriptFile.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs index 80215eeca..4851af99c 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs @@ -22,8 +22,8 @@ public class ScriptFile private static readonly string[] s_newlines = new [] { - "\n", - "\r\n" + "\r\n", + "\n" }; private Token[] scriptTokens; From 50c5fb486d8cc6a6efa0f9c30035a63fe63a05fd Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Wed, 24 Oct 2018 13:30:25 -0700 Subject: [PATCH 4/5] Fix test --- test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs index 4d8ec3f80..aea1da795 100644 --- a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs +++ b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs @@ -257,7 +257,7 @@ public class ScriptFileGetLinesTests { private ScriptFile scriptFile; - private const string TestString = "Line One\r\nLine Two\r\nLine Three\r\nLine Four\r\nLine Five"; + private const string TestString = "Line One\r\nLine Two\r\nLine Three\r\nLine Four\r\nLine Five\r\n"; private readonly string[] TestStringLines = TestString.Split( new string[] { "\r\n" }, From 4429850edf4dab7798dfe57ba9b46affc326f70d Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 29 Oct 2018 14:53:56 -0700 Subject: [PATCH 5/5] Include variant line split tests --- .../Session/ScriptFileTests.cs | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs index aea1da795..adfc2727e 100644 --- a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs +++ b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs @@ -255,26 +255,35 @@ private void AssertFileChange( public class ScriptFileGetLinesTests { - private ScriptFile scriptFile; + private const string TestString_NoTrailingNewline = "Line One\r\nLine Two\r\nLine Three\r\nLine Four\r\nLine Five"; + + private const string TestString_TrailingNewline = TestString_NoTrailingNewline + "\r\n"; + + private static readonly string[] s_newLines = new string[] { "\r\n" }; + + private static readonly string[] s_testStringLines_noTrailingNewline = TestString_NoTrailingNewline.Split(s_newLines, StringSplitOptions.None); + + private static readonly string[] s_testStringLines_trailingNewline = TestString_TrailingNewline.Split(s_newLines, StringSplitOptions.None); + + private ScriptFile _scriptFile_trailingNewline; + + private ScriptFile _scriptFile_noTrailingNewline; - private const string TestString = "Line One\r\nLine Two\r\nLine Three\r\nLine Four\r\nLine Five\r\n"; - private readonly string[] TestStringLines = - TestString.Split( - new string[] { "\r\n" }, - StringSplitOptions.None); public ScriptFileGetLinesTests() { - this.scriptFile = - ScriptFileChangeTests.CreateScriptFile( - "Line One\r\nLine Two\r\nLine Three\r\nLine Four\r\nLine Five\r\n"); + _scriptFile_noTrailingNewline = ScriptFileChangeTests.CreateScriptFile( + TestString_NoTrailingNewline); + + _scriptFile_trailingNewline = ScriptFileChangeTests.CreateScriptFile( + TestString_TrailingNewline); } [Fact] public void CanGetWholeLine() { string[] lines = - this.scriptFile.GetLinesInRange( + _scriptFile_noTrailingNewline.GetLinesInRange( new BufferRange(5, 1, 5, 10)); Assert.Equal(1, lines.Length); @@ -285,17 +294,17 @@ public void CanGetWholeLine() public void CanGetMultipleWholeLines() { string[] lines = - this.scriptFile.GetLinesInRange( + _scriptFile_noTrailingNewline.GetLinesInRange( new BufferRange(2, 1, 4, 10)); - Assert.Equal(TestStringLines.Skip(1).Take(3), lines); + Assert.Equal(s_testStringLines_noTrailingNewline.Skip(1).Take(3), lines); } [Fact] public void CanGetSubstringInSingleLine() { string[] lines = - this.scriptFile.GetLinesInRange( + _scriptFile_noTrailingNewline.GetLinesInRange( new BufferRange(4, 3, 4, 8)); Assert.Equal(1, lines.Length); @@ -306,7 +315,7 @@ public void CanGetSubstringInSingleLine() public void CanGetEmptySubstringRange() { string[] lines = - this.scriptFile.GetLinesInRange( + _scriptFile_noTrailingNewline.GetLinesInRange( new BufferRange(4, 3, 4, 3)); Assert.Equal(1, lines.Length); @@ -324,7 +333,7 @@ public void CanGetSubstringInMultipleLines() }; string[] lines = - this.scriptFile.GetLinesInRange( + _scriptFile_noTrailingNewline.GetLinesInRange( new BufferRange(2, 6, 4, 9)); Assert.Equal(expectedLines, lines); @@ -341,23 +350,29 @@ public void CanGetRangeAtLineBoundaries() }; string[] lines = - this.scriptFile.GetLinesInRange( + _scriptFile_noTrailingNewline.GetLinesInRange( new BufferRange(2, 9, 4, 1)); Assert.Equal(expectedLines, lines); } [Fact] - public void CanSplitLines() + public void CanSplitLines_NoTrailingNewline() + { + Assert.Equal(s_testStringLines_noTrailingNewline, _scriptFile_noTrailingNewline.FileLines); + } + + [Fact] + public void CanSplitLines_TrailingNewline() { - Assert.Equal(TestStringLines, scriptFile.FileLines); + Assert.Equal(s_testStringLines_trailingNewline, _scriptFile_trailingNewline.FileLines); } [Fact] public void CanGetSameLinesWithUnixLineBreaks() { - var unixFile = ScriptFileChangeTests.CreateScriptFile(TestString.Replace("\r\n", "\n")); - Assert.Equal(scriptFile.FileLines, unixFile.FileLines); + var unixFile = ScriptFileChangeTests.CreateScriptFile(TestString_NoTrailingNewline.Replace("\r\n", "\n")); + Assert.Equal(_scriptFile_noTrailingNewline.FileLines, unixFile.FileLines); } [Fact]