Skip to content

Commit 528e3f1

Browse files
thecoolwinterthecoolwinter
authored
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 <!--- Add things that are not yet implemented above --> - [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)
1 parent 160ae95 commit 528e3f1

File tree

6 files changed

+110
-106
lines changed

6 files changed

+110
-106
lines changed

‎Sources/CodeEditSourceEditor/Controller/TextViewController+LoadView.swift‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,15 @@ extension TextViewController {
110110
}
111111
.store(in: &cancellables)
112112

113-
NSEvent.addLocalMonitorForEvents(matching: .keyDown) { event in
114-
guard self.view.window?.firstResponder == self.textView else { return event }
115-
// let charactersIgnoringModifiers = event.charactersIgnoringModifiers
113+
if let localEventMonitor = self.localEvenMonitor {
114+
NSEvent.removeMonitor(localEventMonitor)
115+
}
116+
self.localEvenMonitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { [weak self] event in
117+
guard self?.view.window?.firstResponder == self?.textView else { return event }
116118
let commandKey = NSEvent.ModifierFlags.command.rawValue
117119
let modifierFlags = event.modifierFlags.intersection(.deviceIndependentFlagsMask).rawValue
118120
if modifierFlags == commandKey && event.charactersIgnoringModifiers == "/" {
119-
self.commandSlashCalled()
121+
self?.commandSlashCalled()
120122
return nil
121123
} else {
122124
return event

‎Sources/CodeEditSourceEditor/Controller/TextViewController+StyleViews.swift‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
//
2+
// TextViewController+StyleViews.swift
3+
// CodeEditSourceEditor
4+
//
5+
// Created by Khan Winter on 7/3/24.
6+
//
7+
8+
import AppKit
9+
10+
extension TextViewController {
11+
package func generateParagraphStyle() -> NSMutableParagraphStyle {
12+
// swiftlint:disable:next force_cast
13+
let paragraph = NSParagraphStyle.default.mutableCopy() as! NSMutableParagraphStyle
14+
paragraph.tabStops.removeAll()
15+
paragraph.defaultTabInterval = CGFloat(tabWidth) * fontCharWidth
16+
return paragraph
17+
}
18+
19+
/// Style the text view.
20+
package func styleTextView() {
21+
textView.selectionManager.selectionBackgroundColor = theme.selection
22+
textView.selectionManager.selectedLineBackgroundColor = getThemeBackground()
23+
textView.selectionManager.highlightSelectedLine = isEditable
24+
textView.selectionManager.insertionPointColor = theme.insertionPoint
25+
paragraphStyle = generateParagraphStyle()
26+
textView.typingAttributes = attributesFor(nil)
27+
}
28+
29+
/// Finds the preferred use theme background.
30+
/// - Returns: The background color to use.
31+
private func getThemeBackground() -> NSColor {
32+
if useThemeBackground {
33+
return theme.lineHighlight
34+
}
35+
36+
if systemAppearance == .darkAqua {
37+
return NSColor.quaternaryLabelColor
38+
}
39+
40+
return NSColor.selectedTextBackgroundColor.withSystemEffect(.disabled)
41+
}
42+
43+
/// Style the gutter view.
44+
package func styleGutterView() {
45+
gutterView.frame.origin.y = -scrollView.contentInsets.top
46+
gutterView.selectedLineColor = useThemeBackground ? theme.lineHighlight : systemAppearance == .darkAqua
47+
? NSColor.quaternaryLabelColor
48+
: NSColor.selectedTextBackgroundColor.withSystemEffect(.disabled)
49+
gutterView.highlightSelectedLines = isEditable
50+
gutterView.font = font.rulerFont
51+
gutterView.backgroundColor = useThemeBackground ? theme.background : .textBackgroundColor
52+
if self.isEditable == false {
53+
gutterView.selectedLineTextColor = nil
54+
gutterView.selectedLineColor = .clear
55+
}
56+
}
57+
58+
/// Style the scroll view.
59+
package func styleScrollView() {
60+
guard let scrollView = view as? NSScrollView else { return }
61+
scrollView.drawsBackground = useThemeBackground
62+
scrollView.backgroundColor = useThemeBackground ? theme.background : .clear
63+
if let contentInsets {
64+
scrollView.automaticallyAdjustsContentInsets = false
65+
scrollView.contentInsets = contentInsets
66+
} else {
67+
scrollView.automaticallyAdjustsContentInsets = true
68+
}
69+
scrollView.contentInsets.bottom = (contentInsets?.bottom ?? 0) + bottomContentInsets
70+
}
71+
}

‎Sources/CodeEditSourceEditor/Controller/TextViewController+Shortcuts.swift‎ renamed to ‎Sources/CodeEditSourceEditor/Controller/TextViewController+ToggleComment.swift‎

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ extension TextViewController {
1212
/// Method called when CMD + / key sequence recognized, comments cursor's current line of code
1313
public func commandSlashCalled() {
1414
guard let cursorPosition = cursorPositions.first else {
15-
print("There is no cursor \(#function)")
1615
return
1716
}
1817
// Many languages require a character sequence at the beginning of the line to comment the line.
@@ -33,41 +32,31 @@ extension TextViewController {
3332

3433
/// Toggles comment string at the beginning of a specified line (lineNumber is 1-indexed)
3534
private func toggleCharsAtBeginningOfLine(chars: String, lineNumber: Int) {
36-
guard let lineInfo = textView.layoutManager.textLineForIndex(lineNumber - 1) else {
37-
print("There are no characters/lineInfo \(#function)")
38-
return
39-
}
40-
guard let lineString = textView.textStorage.substring(from: lineInfo.range) else {
41-
print("There are no characters/lineString \(#function)")
35+
guard let lineInfo = textView.layoutManager.textLineForIndex(lineNumber - 1),
36+
let lineString = textView.textStorage.substring(from: lineInfo.range) else {
4237
return
4338
}
4439
let firstNonWhiteSpaceCharIndex = lineString.firstIndex(where: {!$0.isWhitespace}) ?? lineString.startIndex
4540
let numWhitespaceChars = lineString.distance(from: lineString.startIndex, to: firstNonWhiteSpaceCharIndex)
4641
let firstCharsInLine = lineString.suffix(from: firstNonWhiteSpaceCharIndex).prefix(chars.count)
4742
// toggle comment off
4843
if firstCharsInLine == chars {
49-
textView.replaceCharacters(in: NSRange(
50-
location: lineInfo.range.location + numWhitespaceChars,
51-
length: chars.count
52-
), with: "")
53-
}
54-
// toggle comment on
55-
else {
56-
textView.replaceCharacters(in: NSRange(
57-
location: lineInfo.range.location + numWhitespaceChars,
58-
length: 0
59-
), with: chars)
44+
textView.replaceCharacters(
45+
in: NSRange(location: lineInfo.range.location + numWhitespaceChars, length: chars.count),
46+
with: ""
47+
)
48+
} else {
49+
// toggle comment on
50+
textView.replaceCharacters(
51+
in: NSRange(location: lineInfo.range.location + numWhitespaceChars, length: 0),
52+
with: chars
53+
)
6054
}
6155
}
6256

6357
/// Toggles a specific string of characters at the end of a specified line. (lineNumber is 1-indexed)
6458
private func toggleCharsAtEndOfLine(chars: String, lineNumber: Int) {
65-
guard let lineInfo = textView.layoutManager.textLineForIndex(lineNumber - 1) else {
66-
print("There are no characters/lineInfo \(#function)")
67-
return
68-
}
69-
guard let lineString = textView.textStorage.substring(from: lineInfo.range) else {
70-
print("There are no characters/lineString \(#function)")
59+
guard let lineInfo = textView.layoutManager.textLineForIndex(lineNumber - 1), !lineInfo.range.isEmpty else {
7160
return
7261
}
7362
let lineLastCharIndex = lineInfo.range.location + lineInfo.range.length - 1
@@ -79,13 +68,12 @@ extension TextViewController {
7968
let lastCharsInLine = textView.textStorage.substring(from: closeCommentRange)
8069
// toggle comment off
8170
if lastCharsInLine == chars {
82-
textView.replaceCharacters(in: NSRange(
83-
location: lineLastCharIndex - closeCommentLength,
84-
length: closeCommentLength
85-
), with: "")
86-
}
87-
// toggle comment on
88-
else {
71+
textView.replaceCharacters(
72+
in: NSRange(location: lineLastCharIndex - closeCommentLength, length: closeCommentLength),
73+
with: ""
74+
)
75+
} else {
76+
// toggle comment on
8977
textView.replaceCharacters(in: NSRange(location: lineLastCharIndex, length: 0), with: chars)
9078
}
9179
}

‎Sources/CodeEditSourceEditor/Controller/TextViewController.swift‎

Lines changed: 8 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ public class TextViewController: NSViewController {
2828
internal var highlightLayers: [CALayer] = []
2929
internal var systemAppearance: NSAppearance.Name?
3030

31+
package var localEvenMonitor: Any?
3132
package var isPostingCursorNotification: Bool = false
3233

3334
/// The string contents.
@@ -172,15 +173,15 @@ public class TextViewController: NSViewController {
172173
/// This will be `nil` if another highlighter provider is passed to the source editor.
173174
internal(set) public var treeSitterClient: TreeSitterClient?
174175

175-
private var fontCharWidth: CGFloat { (" " as NSString).size(withAttributes: [.font: font]).width }
176+
package var fontCharWidth: CGFloat { (" " as NSString).size(withAttributes: [.font: font]).width }
176177

177178
/// Filters used when applying edits..
178179
internal var textFilters: [TextFormation.Filter] = []
179180

180181
internal var cancellables = Set<AnyCancellable>()
181182

182183
/// ScrollView's bottom inset using as editor overscroll
183-
private var bottomContentInsets: CGFloat {
184+
package var bottomContentInsets: CGFloat {
184185
let height = view.frame.height
185186
var inset = editorOverscroll * height
186187

@@ -270,15 +271,7 @@ public class TextViewController: NSViewController {
270271
// MARK: Paragraph Style
271272

272273
/// A default `NSParagraphStyle` with a set `lineHeight`
273-
internal lazy var paragraphStyle: NSMutableParagraphStyle = generateParagraphStyle()
274-
275-
private func generateParagraphStyle() -> NSMutableParagraphStyle {
276-
// swiftlint:disable:next force_cast
277-
let paragraph = NSParagraphStyle.default.mutableCopy() as! NSMutableParagraphStyle
278-
paragraph.tabStops.removeAll()
279-
paragraph.defaultTabInterval = CGFloat(tabWidth) * fontCharWidth
280-
return paragraph
281-
}
274+
package lazy var paragraphStyle: NSMutableParagraphStyle = generateParagraphStyle()
282275

283276
// MARK: - Reload UI
284277

@@ -293,59 +286,6 @@ public class TextViewController: NSViewController {
293286
highlighter?.invalidate()
294287
}
295288

296-
/// Style the text view.
297-
package func styleTextView() {
298-
textView.selectionManager.selectionBackgroundColor = theme.selection
299-
textView.selectionManager.selectedLineBackgroundColor = getThemeBackground()
300-
textView.selectionManager.highlightSelectedLine = isEditable
301-
textView.selectionManager.insertionPointColor = theme.insertionPoint
302-
paragraphStyle = generateParagraphStyle()
303-
textView.typingAttributes = attributesFor(nil)
304-
}
305-
306-
/// Finds the preferred use theme background.
307-
/// - Returns: The background color to use.
308-
private func getThemeBackground() -> NSColor {
309-
if useThemeBackground {
310-
return theme.lineHighlight
311-
}
312-
313-
if systemAppearance == .darkAqua {
314-
return NSColor.quaternaryLabelColor
315-
}
316-
317-
return NSColor.selectedTextBackgroundColor.withSystemEffect(.disabled)
318-
}
319-
320-
/// Style the gutter view.
321-
package func styleGutterView() {
322-
gutterView.frame.origin.y = -scrollView.contentInsets.top
323-
gutterView.selectedLineColor = useThemeBackground ? theme.lineHighlight : systemAppearance == .darkAqua
324-
? NSColor.quaternaryLabelColor
325-
: NSColor.selectedTextBackgroundColor.withSystemEffect(.disabled)
326-
gutterView.highlightSelectedLines = isEditable
327-
gutterView.font = font.rulerFont
328-
gutterView.backgroundColor = useThemeBackground ? theme.background : .textBackgroundColor
329-
if self.isEditable == false {
330-
gutterView.selectedLineTextColor = nil
331-
gutterView.selectedLineColor = .clear
332-
}
333-
}
334-
335-
/// Style the scroll view.
336-
package func styleScrollView() {
337-
guard let scrollView = view as? NSScrollView else { return }
338-
scrollView.drawsBackground = useThemeBackground
339-
scrollView.backgroundColor = useThemeBackground ? theme.background : .clear
340-
if let contentInsets {
341-
scrollView.automaticallyAdjustsContentInsets = false
342-
scrollView.contentInsets = contentInsets
343-
} else {
344-
scrollView.automaticallyAdjustsContentInsets = true
345-
}
346-
scrollView.contentInsets.bottom = (contentInsets?.bottom ?? 0) + bottomContentInsets
347-
}
348-
349289
deinit {
350290
if let highlighter {
351291
textView.removeStorageDelegate(highlighter)
@@ -354,6 +294,10 @@ public class TextViewController: NSViewController {
354294
highlightProvider = nil
355295
NotificationCenter.default.removeObserver(self)
356296
cancellables.forEach { $0.cancel() }
297+
if let localEvenMonitor {
298+
NSEvent.removeMonitor(localEvenMonitor)
299+
}
300+
localEvenMonitor = nil
357301
}
358302
}
359303

‎Sources/CodeEditSourceEditor/Gutter/GutterView.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,5 +213,7 @@ public class GutterView: NSView {
213213

214214
deinit {
215215
NotificationCenter.default.removeObserver(self)
216+
delegate = nil
217+
textView = nil
216218
}
217219
}

‎Sources/CodeEditSourceEditor/Highlighting/Highlighter.swift‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class Highlighter: NSObject {
4545
private var theme: EditorTheme
4646

4747
/// The object providing attributes for captures.
48-
private weak var attributeProvider: ThemeAttributesProviding!
48+
private weak var attributeProvider: ThemeAttributesProviding?
4949

5050
/// The current language of the editor.
5151
private var language: CodeLanguage
@@ -113,7 +113,7 @@ class Highlighter: NSObject {
113113
// Remove all current highlights. Makes the language setting feel snappier and tells the user we're doing
114114
// something immediately.
115115
textView.textStorage.setAttributes(
116-
attributeProvider.attributesFor(nil),
116+
attributeProvider?.attributesFor(nil) ?? [:],
117117
range: NSRange(location: 0, length: textView.textStorage.length)
118118
)
119119
textView.layoutManager.invalidateLayoutForRect(textView.visibleRect)
@@ -133,6 +133,7 @@ class Highlighter: NSObject {
133133
}
134134

135135
deinit {
136+
NotificationCenter.default.removeObserver(self)
136137
self.attributeProvider = nil
137138
self.textView = nil
138139
self.highlightProvider = nil
@@ -234,10 +235,7 @@ private extension Highlighter {
234235
// they need to be changed back.
235236
for ignoredRange in ignoredIndexes.rangeView
236237
where textView?.documentRange.upperBound ?? 0 > ignoredRange.upperBound {
237-
textView?.textStorage.setAttributes(
238-
attributeProvider.attributesFor(nil),
239-
range: NSRange(ignoredRange)
240-
)
238+
textView?.textStorage.setAttributes(attributeProvider.attributesFor(nil), range: NSRange(ignoredRange))
241239
}
242240

243241
textView?.textStorage.endEditing()
@@ -262,7 +260,6 @@ private extension Highlighter {
262260
length: min(rangeChunkLimit, range.upperBound - range.lowerBound)
263261
)
264262
}
265-
266263
}
267264

268265
// MARK: - Visible Content Updates

0 commit comments

Comments
 (0)