Update to use in-place mutation on buffer.lines to update content

intitni · intitni · commit 07b41a67bb03 · 2022-12-07T23:57:08.000+08:00
So that the editor won't jump to top on undos / redos. Some tests are behaving weird, should find a better way to assert.
diff --git a/Core/Package.swift b/Core/Package.swift
@@ -36,6 +36,10 @@ let package = Package(
             name: "CopilotModel",
             dependencies: ["LanguageClient"]
         ),
+        .testTarget(
+            name: "CopilotModelTests",
+            dependencies: ["CopilotModel"]
+        ),
         .target(
             name: "SuggestionInjector",
             dependencies: ["CopilotModel"]
diff --git a/Core/Sources/CopilotModel/Modification.swift b/Core/Sources/CopilotModel/Modification.swift
@@ -14,7 +14,7 @@ public extension Array where Element == String {
                 let removingRange = range.lowerBound ..< (range.upperBound + 1)
                 removeSubrange(removingRange.clamped(to: 0 ..< endIndex))
             case let .inserted(index, strings):
-                insert(contentsOf: strings, at: index)
+                insert(contentsOf: strings, at: Swift.min(endIndex, index))
             }
         }
     }
@@ -25,3 +25,20 @@ public extension Array where Element == String {
         return newArray
     }
 }
+
+public extension NSMutableArray {
+    func apply(_ modifications: [Modification]) {
+        for modification in modifications {
+            switch modification {
+            case let .deleted(range):
+                if count == 0 { break }
+                let newRange = range.clamped(to: 0 ... (count - 1))
+                removeObjects(in: NSRange(newRange))
+            case let .inserted(index, strings):
+                for string in strings.reversed() {
+                    insert(string, at: Swift.min(count, index))
+                }
+            }
+        }
+    }
+}
diff --git a/Core/Sources/SuggestionInjector/SuggestionInjector.swift b/Core/Sources/SuggestionInjector/SuggestionInjector.swift
@@ -123,19 +123,19 @@ public struct SuggestionInjector {
         let commonPrefix = longestCommonPrefix(of: suggestionContent, and: existedLine ?? "")
 
         if let existedLine, existedLine.count > 1, !commonPrefix.isEmpty {
-            content.remove(at: start.line)
             extraInfo.modifications.append(.deleted(start.line...start.line))
+            content.remove(at: start.line)
         } else if content.count > start.line,
                   content[start.line].isEmpty || content[start.line] == "\n"
         {
-            content.remove(at: start.line)
             extraInfo.modifications.append(.deleted(start.line...start.line))
+            content.remove(at: start.line)
         }
 
         let toBeInserted = suggestionContent.breakLines(appendLineBreakToLastLine: true)
         if content.endIndex < start.line {
-            content.append(contentsOf: toBeInserted)
             extraInfo.modifications.append(.inserted(content.endIndex, toBeInserted))
+            content.append(contentsOf: toBeInserted)
             cursorPosition = .init(
                 line: toBeInserted.endIndex,
                 character: (toBeInserted.last?.count ?? 1) - 1
diff --git a/Core/Tests/CopilotModelTests/ModificationTests.swift b/Core/Tests/CopilotModelTests/ModificationTests.swift
@@ -0,0 +1,37 @@
+import XCTest
+
+@testable import CopilotModel
+
+final class ModificationTests: XCTestCase {
+    func test_nsmutablearray_deleting_an_element() {
+        let a = NSMutableArray(array: ["a", "b", "c"])
+        a.apply([.deleted(0 ... 0)])
+        XCTAssertEqual(a as! [String], ["b", "c"])
+    }
+
+    func test_nsmutablearray_deleting_all_element() {
+        let a = NSMutableArray(array: ["a", "b", "c"])
+        a.apply([.deleted(0 ... 2)])
+        XCTAssertEqual(a as! [String], [])
+    }
+
+    func test_nsmutablearray_deleting_too_much_element() {
+        let a = NSMutableArray(array: ["a", "b", "c"])
+        a.apply([.deleted(0 ... 100)])
+        XCTAssertEqual(a as! [String], [])
+    }
+
+    func test_nsmutablearray_inserting_elements() {
+        let a = NSMutableArray(array: ["a", "b", "c"])
+        a.apply([.inserted(0, ["y", "z"])])
+        XCTAssertEqual(a as! [String], ["y", "z", "a", "b", "c"])
+        a.apply([.inserted(1, ["0", "1"])])
+        XCTAssertEqual(a as! [String], ["y", "0", "1", "z", "a", "b", "c"])
+    }
+
+    func test_nsmutablearray_inserting_elements_at_index_out_of_range() {
+        let a = NSMutableArray(array: ["a", "b", "c"])
+        a.apply([.inserted(1000, ["z"])])
+        XCTAssertEqual(a as! [String], ["a", "b", "c", "z"])
+    }
+}
diff --git a/EditorExtension/Helpers.swift b/EditorExtension/Helpers.swift
@@ -3,30 +3,33 @@ import Foundation
 import XcodeKit
 
 extension XCSourceEditorCommandInvocation {
-    func mutateCompleteBuffer(content: String, restoringSelections restore: Bool) {
+    func mutateCompleteBuffer(modifications: [Modification], restoringSelections restore: Bool) {
         if restore {
             let selectionsRangesToRestore = buffer.selections.compactMap { $0 as? XCSourceTextRange }
             buffer.selections.removeAllObjects()
-            buffer.completeBuffer = content
+            buffer.lines.apply(modifications)
             for range in selectionsRangesToRestore {
                 buffer.selections.add(range)
             }
         } else {
-            buffer.completeBuffer = content
+            buffer.lines.apply(modifications)
         }
     }
 
     func accept(_ updatedContent: UpdatedContent) {
         if let newCursor = updatedContent.newCursor {
-            mutateCompleteBuffer(content: updatedContent.content, restoringSelections: false)
+            mutateCompleteBuffer(
+                modifications: updatedContent.modifications,
+                restoringSelections: false
+            )
             buffer.selections.removeAllObjects()
             buffer.selections.add(XCSourceTextRange(
                 start: .init(line: newCursor.line, column: newCursor.character),
                 end: .init(line: newCursor.line, column: newCursor.character)
             ))
         } else {
             mutateCompleteBuffer(
-                content: updatedContent.content,
+                modifications: updatedContent.modifications,
                 restoringSelections: true
             )
         }
diff --git a/XCPServiceTests/AcceptSuggestionTests.swift b/XCPServiceTests/AcceptSuggestionTests.swift
@@ -20,8 +20,8 @@ final class AcceptSuggestionTests: XCTestCase {
                 struct Dog {}
                 """,
                 range: .init(
-                    start: .init(line: 7, character: 0),
-                    end: .init(line: 7, character: 12)
+                    start: .init(line: 1, character: 0),
+                    end: .init(line: 1, character: 12)
                 )
             ),
         ]
@@ -33,7 +33,7 @@ final class AcceptSuggestionTests: XCTestCase {
 
         let result1 = try await service.getSuggestedCode(editorContent: .init(
             content: content,
-            lines: content.breakLines(),
+            lines: content.breakLines(appendLineBreakToLastLine: true),
             uti: "",
             cursorPosition: .init(line: 0, character: 0),
             tabSize: 1,
@@ -43,18 +43,23 @@ final class AcceptSuggestionTests: XCTestCase {
 
         let result2 = try await service.getSuggestionAcceptedCode(editorContent: .init(
             content: result1.content,
-            lines: result1.content.breakLines(),
+            lines: result1.content.breakLines(appendLineBreakToLastLine: true),
             uti: "",
             cursorPosition: .init(line: 3, character: 5),
             tabSize: 1,
             indentSize: 1,
             usesTabsForIndentation: false
         ))
 
+        XCTAssertEqual(
+            Array(result2.content.breakLines(appendLineBreakToLastLine: true).dropLast(1)),
+            result1.content.breakLines(appendLineBreakToLastLine: true).applying(result2.modifications)
+        )
         XCTAssertEqual(result2.content, """
         struct Cat {}
 
         struct Dog {}
+        
 
         """, "Previous suggestions should be removed.")
 
@@ -66,7 +71,7 @@ final class AcceptSuggestionTests: XCTestCase {
 
         let result3 = try await service.getSuggestionAcceptedCode(editorContent: .init(
             content: content,
-            lines: content.breakLines(),
+            lines: content.breakLines(appendLineBreakToLastLine: true),
             uti: "",
             cursorPosition: .init(line: 0, character: 3),
             tabSize: 1,
@@ -75,7 +80,10 @@ final class AcceptSuggestionTests: XCTestCase {
         ))
 
         XCTAssertEqual(result3.content, content, "Deleting the code and accept again does nothing")
-
+        XCTAssertEqual(
+            result3.content.breakLines(appendLineBreakToLastLine: true),
+            content.breakLines(appendLineBreakToLastLine: true).applying(result3.modifications)
+        )
         XCTAssertEqual(result3.newCursor, nil)
     }
 }
diff --git a/XCPServiceTests/GetSuggestionsTests.swift b/XCPServiceTests/GetSuggestionsTests.swift
@@ -43,6 +43,10 @@ final class GetSuggestionsTests: XCTestCase {
             usesTabsForIndentation: false
         ))
 
+        XCTAssertEqual(
+            result.content.breakLines(appendLineBreakToLastLine: true),
+            content.breakLines(appendLineBreakToLastLine: true).applying(result.modifications)
+        )
         XCTAssertEqual(result.content, """
         struct Cat {
 
@@ -92,7 +96,10 @@ final class GetSuggestionsTests: XCTestCase {
             indentSize: 1,
             usesTabsForIndentation: false
         ))
-
+        XCTAssertEqual(
+            result.content.breakLines(appendLineBreakToLastLine: true),
+            content.breakLines(appendLineBreakToLastLine: true).applying(result.modifications)
+        )
         XCTAssertEqual(result.content, """
         struct Cat {
 
diff --git a/XPCService/Shared/Models.swift b/XPCService/Shared/Models.swift
@@ -14,4 +14,5 @@ struct EditorContent: Codable {
 struct UpdatedContent: Codable {
     var content: String
     var newCursor: CursorPosition?
+    var modifications: [Modification]
 }
diff --git a/XPCService/Workspace.swift b/XPCService/Workspace.swift
diff --git a/XPCService/XPCService.swift b/XPCService/XPCService.swift

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ public extension Array where Element == String {`
`14`	`14`	`let removingRange = range.lowerBound ..< (range.upperBound + 1)`
`15`	`15`	`removeSubrange(removingRange.clamped(to: 0 ..< endIndex))`
`16`	`16`	`case let .inserted(index, strings):`
`17`		`- insert(contentsOf: strings, at: index)`
	`17`	`+ insert(contentsOf: strings, at: Swift.min(endIndex, index))`
`18`	`18`	`}`
`19`	`19`	`}`
`20`	`20`	`}`
`@@ -25,3 +25,20 @@ public extension Array where Element == String {`
`25`	`25`	`return newArray`
`26`	`26`	`}`
`27`	`27`	`}`
	`28`	`+`
	`29`	`+public extension NSMutableArray {`
	`30`	`+ func apply(_ modifications: [Modification]) {`
	`31`	`+ for modification in modifications {`
	`32`	`+ switch modification {`
	`33`	`+ case let .deleted(range):`
	`34`	`+ if count == 0 { break }`
	`35`	`+ let newRange = range.clamped(to: 0 ... (count - 1))`
	`36`	`+ removeObjects(in: NSRange(newRange))`
	`37`	`+ case let .inserted(index, strings):`
	`38`	`+ for string in strings.reversed() {`
	`39`	`+ insert(string, at: Swift.min(count, index))`
	`40`	`+ }`
	`41`	`+ }`
	`42`	`+ }`
	`43`	`+ }`
	`44`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -14,4 +14,5 @@ struct EditorContent: Codable {`
`14`	`14`	`struct UpdatedContent: Codable {`
`15`	`15`	`var content: String`
`16`	`16`	`var newCursor: CursorPosition?`
	`17`	`+ var modifications: [Modification]`
`17`	`18`	`}`