Fix Reference Cycle (#255)

### Description Fixes a strong reference cycle in `TextViewController` causing it to not release when removed from the view heirarchy. ### Related Issues * CodeEditApp/CodeEdit#1794 ### Checklist  - [x] I read and understood the [contributing guide](https://github.com/CodeEditApp/CodeEdit/blob/main/CONTRIBUTING.md) as well as the [code of conduct](https://github.com/CodeEditApp/CodeEdit/blob/main/CODE_OF_CONDUCT.md) - [x] The issues this PR addresses are related to each other - [x] My changes generate no new warnings - [x] My code builds and runs on my machine - [x] My changes are all related to the related issue above - [x] I documented my code ### Screenshots Stable memory usage in the example app (mem jumps are opening & closing a large file a few times): ![Screenshot 2024-07-03 at 8 22 20 PM](https://github.com/CodeEditApp/CodeEditSourceEditor/assets/35942988/7532e0f5-f72e-44b5-ac8c-5f194736d3d4)
CodeEditApp · Jul 4, 2024 · 528e3f1 · 528e3f1
1 parent 160ae95
commit 528e3f1
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 106 deletions.
diff --git a/Sources/CodeEditSourceEditor/Controller/TextViewController+LoadView.swift b/Sources/CodeEditSourceEditor/Controller/TextViewController+LoadView.swift
@@ -110,13 +110,15 @@ extension TextViewController {
             }
             .store(in: &cancellables)
 
-        NSEvent.addLocalMonitorForEvents(matching: .keyDown) { event in
-            guard self.view.window?.firstResponder == self.textView else { return event }
-//            let charactersIgnoringModifiers = event.charactersIgnoringModifiers
+        if let localEventMonitor = self.localEvenMonitor {
+            NSEvent.removeMonitor(localEventMonitor)
+        }
+        self.localEvenMonitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { [weak self] event in
+            guard self?.view.window?.firstResponder == self?.textView else { return event }
             let commandKey = NSEvent.ModifierFlags.command.rawValue
             let modifierFlags = event.modifierFlags.intersection(.deviceIndependentFlagsMask).rawValue
             if modifierFlags == commandKey && event.charactersIgnoringModifiers == "/" {
-                self.commandSlashCalled()
+                self?.commandSlashCalled()
                 return nil
             } else {
                 return event

diff --git a/Sources/CodeEditSourceEditor/Controller/TextViewController+StyleViews.swift b/Sources/CodeEditSourceEditor/Controller/TextViewController+StyleViews.swift
@@ -0,0 +1,71 @@
+//
+//  TextViewController+StyleViews.swift
+//  CodeEditSourceEditor
+//
+//  Created by Khan Winter on 7/3/24.
+//
+
+import AppKit
+
+extension TextViewController {
+    package func generateParagraphStyle() -> NSMutableParagraphStyle {
+        // swiftlint:disable:next force_cast
+        let paragraph = NSParagraphStyle.default.mutableCopy() as! NSMutableParagraphStyle
+        paragraph.tabStops.removeAll()
+        paragraph.defaultTabInterval = CGFloat(tabWidth) * fontCharWidth
+        return paragraph
+    }
+
+    /// Style the text view.
+    package func styleTextView() {
+        textView.selectionManager.selectionBackgroundColor = theme.selection
+        textView.selectionManager.selectedLineBackgroundColor = getThemeBackground()
+        textView.selectionManager.highlightSelectedLine = isEditable
+        textView.selectionManager.insertionPointColor = theme.insertionPoint
+        paragraphStyle = generateParagraphStyle()
+        textView.typingAttributes = attributesFor(nil)
+    }
+
+    /// Finds the preferred use theme background.
+    /// - Returns: The background color to use.
+    private func getThemeBackground() -> NSColor {
+        if useThemeBackground {
+            return theme.lineHighlight
+        }
+
+        if systemAppearance == .darkAqua {
+            return NSColor.quaternaryLabelColor
+        }
+
+        return NSColor.selectedTextBackgroundColor.withSystemEffect(.disabled)
+    }
+
+    /// Style the gutter view.
+    package func styleGutterView() {
+        gutterView.frame.origin.y = -scrollView.contentInsets.top
+        gutterView.selectedLineColor = useThemeBackground ? theme.lineHighlight : systemAppearance == .darkAqua
+        ? NSColor.quaternaryLabelColor
+        : NSColor.selectedTextBackgroundColor.withSystemEffect(.disabled)
+        gutterView.highlightSelectedLines = isEditable
+        gutterView.font = font.rulerFont
+        gutterView.backgroundColor = useThemeBackground ? theme.background : .textBackgroundColor
+        if self.isEditable == false {
+            gutterView.selectedLineTextColor = nil
+            gutterView.selectedLineColor = .clear
+        }
+    }
+
+    /// Style the scroll view.
+    package func styleScrollView() {
+        guard let scrollView = view as? NSScrollView else { return }
+        scrollView.drawsBackground = useThemeBackground
+        scrollView.backgroundColor = useThemeBackground ? theme.background : .clear
+        if let contentInsets {
+            scrollView.automaticallyAdjustsContentInsets = false
+            scrollView.contentInsets = contentInsets
+        } else {
+            scrollView.automaticallyAdjustsContentInsets = true
+        }
+        scrollView.contentInsets.bottom = (contentInsets?.bottom ?? 0) + bottomContentInsets
+    }
+}
diff --git a/...roller/TextViewController+Shortcuts.swift → ...er/TextViewController+ToggleComment.swift b/...roller/TextViewController+Shortcuts.swift → ...er/TextViewController+ToggleComment.swift
@@ -12,7 +12,6 @@ extension TextViewController {
     /// Method called when CMD + / key sequence recognized, comments cursor's current line of code
     public func commandSlashCalled() {
         guard let cursorPosition = cursorPositions.first else {
-            print("There is no cursor \(#function)")
             return
         }
         // Many languages require a character sequence at the beginning of the line to comment the line.
@@ -33,41 +32,31 @@ extension TextViewController {
 
     ///  Toggles comment string at the beginning of a specified line (lineNumber is 1-indexed)
     private func toggleCharsAtBeginningOfLine(chars: String, lineNumber: Int) {
-        guard let lineInfo = textView.layoutManager.textLineForIndex(lineNumber - 1) else {
-            print("There are no characters/lineInfo \(#function)")
-            return
-        }
-        guard let lineString = textView.textStorage.substring(from: lineInfo.range) else {
-            print("There are no characters/lineString \(#function)")
+        guard let lineInfo = textView.layoutManager.textLineForIndex(lineNumber - 1),
+              let lineString = textView.textStorage.substring(from: lineInfo.range) else {
             return
         }
         let firstNonWhiteSpaceCharIndex = lineString.firstIndex(where: {!$0.isWhitespace}) ?? lineString.startIndex
         let numWhitespaceChars = lineString.distance(from: lineString.startIndex, to: firstNonWhiteSpaceCharIndex)
         let firstCharsInLine = lineString.suffix(from: firstNonWhiteSpaceCharIndex).prefix(chars.count)
         // toggle comment off
         if firstCharsInLine == chars {
-            textView.replaceCharacters(in: NSRange(
-                location: lineInfo.range.location + numWhitespaceChars,
-                length: chars.count
-            ), with: "")
-        }
-        // toggle comment on
-        else {
-            textView.replaceCharacters(in: NSRange(
-                location: lineInfo.range.location + numWhitespaceChars,
-                length: 0
-            ), with: chars)
+            textView.replaceCharacters(
+                in: NSRange(location: lineInfo.range.location + numWhitespaceChars, length: chars.count),
+                with: ""
+            )
+        } else {
+            // toggle comment on
+            textView.replaceCharacters(
+                in: NSRange(location: lineInfo.range.location + numWhitespaceChars, length: 0),
+                with: chars
+            )
         }
     }
 
     ///  Toggles a specific string of characters at the end of a specified line. (lineNumber is 1-indexed)
     private func toggleCharsAtEndOfLine(chars: String, lineNumber: Int) {
-        guard let lineInfo = textView.layoutManager.textLineForIndex(lineNumber - 1) else {
-            print("There are no characters/lineInfo \(#function)")
-            return
-        }
-        guard let lineString = textView.textStorage.substring(from: lineInfo.range) else {
-            print("There are no characters/lineString \(#function)")
+        guard let lineInfo = textView.layoutManager.textLineForIndex(lineNumber - 1), !lineInfo.range.isEmpty else {
             return
         }
         let lineLastCharIndex = lineInfo.range.location + lineInfo.range.length - 1
@@ -79,13 +68,12 @@ extension TextViewController {
         let lastCharsInLine = textView.textStorage.substring(from: closeCommentRange)
         // toggle comment off
         if lastCharsInLine == chars {
-            textView.replaceCharacters(in: NSRange(
-                location: lineLastCharIndex - closeCommentLength,
-                length: closeCommentLength
-            ), with: "")
-        }
-        // toggle comment on
-        else {
+            textView.replaceCharacters(
+                in: NSRange(location: lineLastCharIndex - closeCommentLength, length: closeCommentLength),
+                with: ""
+            )
+        } else {
+            // toggle comment on
             textView.replaceCharacters(in: NSRange(location: lineLastCharIndex, length: 0), with: chars)
         }
     }

diff --git a/Sources/CodeEditSourceEditor/Controller/TextViewController.swift b/Sources/CodeEditSourceEditor/Controller/TextViewController.swift
@@ -28,6 +28,7 @@ public class TextViewController: NSViewController {
     internal var highlightLayers: [CALayer] = []
     internal var systemAppearance: NSAppearance.Name?
 
+    package var localEvenMonitor: Any?
     package var isPostingCursorNotification: Bool = false
 
     /// The string contents.
@@ -172,15 +173,15 @@ public class TextViewController: NSViewController {
     /// This will be `nil` if another highlighter provider is passed to the source editor.
     internal(set) public var treeSitterClient: TreeSitterClient?
 
-    private var fontCharWidth: CGFloat { (" " as NSString).size(withAttributes: [.font: font]).width }
+    package var fontCharWidth: CGFloat { (" " as NSString).size(withAttributes: [.font: font]).width }
 
     /// Filters used when applying edits..
     internal var textFilters: [TextFormation.Filter] = []
 
     internal var cancellables = Set<AnyCancellable>()
 
     /// ScrollView's bottom inset using as editor overscroll
-    private var bottomContentInsets: CGFloat {
+    package var bottomContentInsets: CGFloat {
         let height = view.frame.height
         var inset = editorOverscroll * height
 
@@ -270,15 +271,7 @@ public class TextViewController: NSViewController {
     // MARK: Paragraph Style
 
     /// A default `NSParagraphStyle` with a set `lineHeight`
-    internal lazy var paragraphStyle: NSMutableParagraphStyle = generateParagraphStyle()
-
-    private func generateParagraphStyle() -> NSMutableParagraphStyle {
-        // swiftlint:disable:next force_cast
-        let paragraph = NSParagraphStyle.default.mutableCopy() as! NSMutableParagraphStyle
-        paragraph.tabStops.removeAll()
-        paragraph.defaultTabInterval = CGFloat(tabWidth) * fontCharWidth
-        return paragraph
-    }
+    package lazy var paragraphStyle: NSMutableParagraphStyle = generateParagraphStyle()
 
     // MARK: - Reload UI
 
@@ -293,59 +286,6 @@ public class TextViewController: NSViewController {
         highlighter?.invalidate()
     }
 
-    /// Style the text view.
-    package func styleTextView() {
-        textView.selectionManager.selectionBackgroundColor = theme.selection
-        textView.selectionManager.selectedLineBackgroundColor = getThemeBackground()
-        textView.selectionManager.highlightSelectedLine = isEditable
-        textView.selectionManager.insertionPointColor = theme.insertionPoint
-        paragraphStyle = generateParagraphStyle()
-        textView.typingAttributes = attributesFor(nil)
-    }
-
-    /// Finds the preferred use theme background.
-    /// - Returns: The background color to use.
-    private func getThemeBackground() -> NSColor {
-        if useThemeBackground {
-            return theme.lineHighlight
-        }
-
-        if systemAppearance == .darkAqua {
-            return NSColor.quaternaryLabelColor
-        }
-
-        return NSColor.selectedTextBackgroundColor.withSystemEffect(.disabled)
-    }
-
-    /// Style the gutter view.
-    package func styleGutterView() {
-        gutterView.frame.origin.y = -scrollView.contentInsets.top
-        gutterView.selectedLineColor = useThemeBackground ? theme.lineHighlight : systemAppearance == .darkAqua
-        ? NSColor.quaternaryLabelColor
-        : NSColor.selectedTextBackgroundColor.withSystemEffect(.disabled)
-        gutterView.highlightSelectedLines = isEditable
-        gutterView.font = font.rulerFont
-        gutterView.backgroundColor = useThemeBackground ? theme.background : .textBackgroundColor
-        if self.isEditable == false {
-            gutterView.selectedLineTextColor = nil
-            gutterView.selectedLineColor = .clear
-        }
-    }
-
-    /// Style the scroll view.
-    package func styleScrollView() {
-        guard let scrollView = view as? NSScrollView else { return }
-        scrollView.drawsBackground = useThemeBackground
-        scrollView.backgroundColor = useThemeBackground ? theme.background : .clear
-        if let contentInsets {
-            scrollView.automaticallyAdjustsContentInsets = false
-            scrollView.contentInsets = contentInsets
-        } else {
-            scrollView.automaticallyAdjustsContentInsets = true
-        }
-        scrollView.contentInsets.bottom = (contentInsets?.bottom ?? 0) + bottomContentInsets
-    }
-
     deinit {
         if let highlighter {
             textView.removeStorageDelegate(highlighter)
@@ -354,6 +294,10 @@ public class TextViewController: NSViewController {
         highlightProvider = nil
         NotificationCenter.default.removeObserver(self)
         cancellables.forEach { $0.cancel() }
+        if let localEvenMonitor {
+            NSEvent.removeMonitor(localEvenMonitor)
+        }
+        localEvenMonitor = nil
     }
 }
 

diff --git a/Sources/CodeEditSourceEditor/Gutter/GutterView.swift b/Sources/CodeEditSourceEditor/Gutter/GutterView.swift
@@ -213,5 +213,7 @@ public class GutterView: NSView {
 
     deinit {
         NotificationCenter.default.removeObserver(self)
+        delegate = nil
+        textView = nil
     }
 }
diff --git a/Sources/CodeEditSourceEditor/Highlighting/Highlighter.swift b/Sources/CodeEditSourceEditor/Highlighting/Highlighter.swift
@@ -45,7 +45,7 @@ class Highlighter: NSObject {
     private var theme: EditorTheme
 
     /// The object providing attributes for captures.
-    private weak var attributeProvider: ThemeAttributesProviding!
+    private weak var attributeProvider: ThemeAttributesProviding?
 
     /// The current language of the editor.
     private var language: CodeLanguage
@@ -113,7 +113,7 @@ class Highlighter: NSObject {
         // Remove all current highlights. Makes the language setting feel snappier and tells the user we're doing
         // something immediately.
         textView.textStorage.setAttributes(
-            attributeProvider.attributesFor(nil),
+            attributeProvider?.attributesFor(nil) ?? [:],
             range: NSRange(location: 0, length: textView.textStorage.length)
         )
         textView.layoutManager.invalidateLayoutForRect(textView.visibleRect)
@@ -133,6 +133,7 @@ class Highlighter: NSObject {
     }
 
     deinit {
+        NotificationCenter.default.removeObserver(self)
         self.attributeProvider = nil
         self.textView = nil
         self.highlightProvider = nil
@@ -234,10 +235,7 @@ private extension Highlighter {
         // they need to be changed back.
         for ignoredRange in ignoredIndexes.rangeView
         where textView?.documentRange.upperBound ?? 0 > ignoredRange.upperBound {
-            textView?.textStorage.setAttributes(
-                attributeProvider.attributesFor(nil),
-                range: NSRange(ignoredRange)
-            )
+            textView?.textStorage.setAttributes(attributeProvider.attributesFor(nil), range: NSRange(ignoredRange))
         }
 
         textView?.textStorage.endEditing()
@@ -262,7 +260,6 @@ private extension Highlighter {
             length: min(rangeChunkLimit, range.upperBound - range.lowerBound)
         )
     }
-
 }
 
 // MARK: - Visible Content Updates