From e9ace8f4d2dd1d2fe9dccf7cbb51534d3f90b358 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 9 Oct 2018 12:45:34 -0700 Subject: [PATCH 1/2] Fix crash where edits up to the end of the file do not work --- .../Workspace/ScriptFile.cs | 214 +++++++++++------- .../Session/ScriptFileTests.cs | 49 ++++ 2 files changed, 182 insertions(+), 81 deletions(-) diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs index e8cb7aac1..802e004da 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs @@ -358,89 +358,11 @@ public void ValidatePosition(int line, int column, bool isInsertion) /// The FileChange to apply to the file's contents. public void ApplyChange(FileChange fileChange) { - // Break up the change lines - string[] changeLines = fileChange.InsertString.Split('\n'); - - if (fileChange.IsReload) - { - this.FileLines.Clear(); - foreach (var changeLine in changeLines) - { - this.FileLines.Add(changeLine); - } - } - else - { - // VSCode sometimes likes to give the change start line as (FileLines.Count + 1). - // This used to crash EditorServices, but we now treat it as an append. - // See https://github.com/PowerShell/vscode-powershell/issues/1283 - if (fileChange.Line == this.FileLines.Count + 1) - { - foreach (string addedLine in changeLines) - { - string finalLine = addedLine.TrimEnd('\r'); - this.FileLines.Add(finalLine); - } - } - // Similarly, when lines are deleted from the end of the file, - // VSCode likes to give the end line as (FileLines.Count + 1). - else if (fileChange.EndLine == this.FileLines.Count + 1 && String.Empty.Equals(fileChange.InsertString)) - { - int lineIndex = fileChange.Line - 1; - this.FileLines.RemoveRange(lineIndex, this.FileLines.Count - lineIndex); - } - // Otherwise, the change needs to go between existing content - else - { - this.ValidatePosition(fileChange.Line, fileChange.Offset); - this.ValidatePosition(fileChange.EndLine, fileChange.EndOffset); - - // Get the first fragment of the first line - string firstLineFragment = - this.FileLines[fileChange.Line - 1] - .Substring(0, fileChange.Offset - 1); - - // Get the last fragment of the last line - string endLine = this.FileLines[fileChange.EndLine - 1]; - string lastLineFragment = - endLine.Substring( - fileChange.EndOffset - 1, - (this.FileLines[fileChange.EndLine - 1].Length - fileChange.EndOffset) + 1); - - // Remove the old lines - for (int i = 0; i <= fileChange.EndLine - fileChange.Line; i++) - { - this.FileLines.RemoveAt(fileChange.Line - 1); - } - - // Build and insert the new lines - int currentLineNumber = fileChange.Line; - for (int changeIndex = 0; changeIndex < changeLines.Length; changeIndex++) - { - // Since we split the lines above using \n, make sure to - // trim the ending \r's off as well. - string finalLine = changeLines[changeIndex].TrimEnd('\r'); - - // Should we add first or last line fragments? - if (changeIndex == 0) - { - // Append the first line fragment - finalLine = firstLineFragment + finalLine; - } - if (changeIndex == changeLines.Length - 1) - { - // Append the last line fragment - finalLine = finalLine + lastLineFragment; - } - - this.FileLines.Insert(currentLineNumber - 1, finalLine); - currentLineNumber++; - } - } - } + // Apply the file change to the underlying file line data structure we use + ApplyChangeToFileLines(fileChange); // Parse the script again to be up-to-date - this.ParseFileContents(); + ParseFileContents(); } /// @@ -587,6 +509,136 @@ private void SetFileContents(string fileContents) this.ParseFileContents(); } + private void ApplyChangeToFileLines(FileChange fileChange) + { + // Break up the change lines + string[] changeLines = fileChange.InsertString.Split('\n'); + for (int i = 0; i < changeLines.Length; i++) + { + changeLines[i] = changeLines[i].TrimEnd('\r'); + } + + if (fileChange.IsReload) + { + this.FileLines.Clear(); + foreach (var changeLine in changeLines) + { + this.FileLines.Add(changeLine); + } + return; + } + + // VSCode sometimes likes to give the change start line as (FileLines.Count + 1). + // This used to crash EditorServices, but we now treat it as an append. + // See https://github.com/PowerShell/vscode-powershell/issues/1283 + if (fileChange.Line == this.FileLines.Count + 1) + { + foreach (string addedLine in changeLines) + { + this.FileLines.Add(addedLine); + } + return; + } + + // Similarly, when lines are deleted or edited from the end of the file, + // VSCode likes to give the end line as (FileLines.Count + 1). + if (fileChange.EndLine == this.FileLines.Count + 1) + { + int lineIndex = fileChange.Line - 1; + + // If the "edit" is just a deletion, make a small optimization + if (String.Empty.Equals(fileChange.InsertString)) + { + this.FileLines.RemoveRange(lineIndex, this.FileLines.Count - lineIndex); + return; + } + + // If we have a true edit, we need to: + // 1. Play the new lines over the old ones starting from the insertion point + // 2. Remove any lines beyond that + + int i = lineIndex; + for (int j = 0; j < changeLines.Length; j++, i++) + { + string addedLine; + if (i == lineIndex && fileChange.Offset != 1) + { + // For the first line, we want to tack on the edit after the offset point + addedLine = this.FileLines[i].Substring(0, fileChange.Offset - 1) + changeLines[j]; + } + else + { + addedLine = changeLines[j]; + } + + if (i < this.FileLines.Count) + { + // If the line is within the range of current lines, we overwrite + this.FileLines[i] = addedLine; + } + else + { + // Otherwise we append to the end + this.FileLines.Add(addedLine); + } + } + + // We may end up with less lines than originally + // In this case, remove the dangling lines + if (i < this.FileLines.Count) + { + this.FileLines.RemoveRange(i, this.FileLines.Count - i); + } + + return; + } + + // Otherwise, the change needs to go between existing content + this.ValidatePosition(fileChange.Line, fileChange.Offset); + this.ValidatePosition(fileChange.EndLine, fileChange.EndOffset); + + // Get the first fragment of the first line + string firstLineFragment = + this.FileLines[fileChange.Line - 1] + .Substring(0, fileChange.Offset - 1); + + // Get the last fragment of the last line + string endLine = this.FileLines[fileChange.EndLine - 1]; + string lastLineFragment = + endLine.Substring( + fileChange.EndOffset - 1, + (this.FileLines[fileChange.EndLine - 1].Length - fileChange.EndOffset) + 1); + + // Remove the old lines + for (int i = 0; i <= fileChange.EndLine - fileChange.Line; i++) + { + this.FileLines.RemoveAt(fileChange.Line - 1); + } + + // Build and insert the new lines + int currentLineNumber = fileChange.Line; + for (int changeIndex = 0; changeIndex < changeLines.Length; changeIndex++) + { + string finalLine = changeLines[changeIndex]; + + // Should we add first or last line fragments? + if (changeIndex == 0) + { + // Append the first line fragment + finalLine = firstLineFragment + finalLine; + } + if (changeIndex == changeLines.Length - 1) + { + // Append the last line fragment + finalLine = finalLine + lastLineFragment; + } + + this.FileLines.Insert(currentLineNumber - 1, finalLine); + currentLineNumber++; + } + + } + /// /// Parses the current file contents to get the AST, tokens, /// and parse errors. diff --git a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs index 4d8ec3f80..91f5be497 100644 --- a/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs +++ b/test/PowerShellEditorServices.Test/Session/ScriptFileTests.cs @@ -223,6 +223,55 @@ public void CanDeleteFromEndOfFile() ); } + [Fact] + public void CanEditUpToEndOfFile() + { + this.AssertFileChange( + "line1\r\nline2\r\nline3\r\nline4", + "line1\r\nline2\r\nnewline3\r\nnewline4", + new FileChange + { + Line = 3, + EndLine = 5, + Offset = 1, + EndOffset = 1, + InsertString = "newline3\r\nnewline4" + } + ); + } + + [Fact] + public void CanEditOffPartOfLineUpToEndOfFile() + { + this.AssertFileChange( + "line1\r\nline2\r\nline3\r\nline4", + "line1\r\nline2\r\nline-new-3\r\nline-new-4\r\nline-new-5", + new FileChange + { + Line = 3, + EndLine = 5, + Offset = 5, + InsertString = "-new-3\r\nline-new-4\r\nline-new-5" + } + ); + } + + [Fact] + public void CanEditSingleLineAtEndOfFile() + { + this.AssertFileChange( + "line1\r\nline2\r\nline3", + "line1\r\nline2\r\nline-new-3", + new FileChange + { + Line = 3, + EndLine = 4, + Offset = 5, + InsertString = "-new-3" + } + ); + } + internal static ScriptFile CreateScriptFile(string initialString) { using (StringReader stringReader = new StringReader(initialString)) From a27cbcc7a88f20d1e28f2f201d404ee1f3d6d868 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 9 Oct 2018 12:51:06 -0700 Subject: [PATCH 2/2] Delete unused newline --- src/PowerShellEditorServices/Workspace/ScriptFile.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/PowerShellEditorServices/Workspace/ScriptFile.cs b/src/PowerShellEditorServices/Workspace/ScriptFile.cs index 802e004da..cbae66c09 100644 --- a/src/PowerShellEditorServices/Workspace/ScriptFile.cs +++ b/src/PowerShellEditorServices/Workspace/ScriptFile.cs @@ -636,7 +636,6 @@ private void ApplyChangeToFileLines(FileChange fileChange) this.FileLines.Insert(currentLineNumber - 1, finalLine); currentLineNumber++; } - } ///