Skip to content

(WIP) Large note processing fix #1224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 76 additions & 31 deletions Simplenote/NSTextView+Simplenote.swift
Original file line number Diff line number Diff line change
Expand Up @@ -145,39 +145,84 @@ extension NSTextView {
///
@objc
func processLinksInDocumentAsynchronously() {
DispatchQueue.main.async(execute: processLinksInDocument)
}

/// Processess Links in the document
///
/// - Important: This API temporarily disables the `delegate`.
/// - Note: Invoking `checkTextInDocument` results in a call to`delegate.textDidChange`.
/// This causes the Editor to update the Note's Modification Date, and may affect the List Sort Order (!)
///
func processLinksInDocument() {
/// Disable the Delegate:
let theDelegate = delegate
delegate = nil

/// Issue #472: Linkification should not be undoable
undoManager?.disableUndoRegistration()

if let textStorage = textStorage as? Storage {

// checkTextInDocument calculate bounds for links and we need to ensure that layout is current before calling beginEditing
// otherwise it will crash trying to update layout inside beginEditing / endEditing block
ensureLayout()
textStorage.beginEditing()
checkTextInDocument(nil)
textStorage.endEditingWithoutRestyling()
} else {
checkTextInDocument(nil)
// Capture the current text and attributes to process in background
guard let textStorage = textStorage,
let attributedString = try? NSAttributedString(attributedString: textStorage.copy() as! NSAttributedString) else {
return
}

undoManager?.enableUndoRegistration()

/// Restore the Delegate
delegate = theDelegate
// Capture the current content hash to detect changes
let contentHash = attributedString.string.hash

DispatchQueue.global(qos: .userInitiated).async { [weak self] in
// Create a data detector for links
let types = NSTextCheckingResult.CheckingType.link.rawValue
guard let detector = try? NSDataDetector(types: types) else {
return
}

// Find all links in the text
let fullRange = NSRange(location: 0, length: attributedString.length)
let matches = detector.matches(in: attributedString.string, options: [], range: fullRange)

// Apply the changes on the main queue
DispatchQueue.main.async { [weak self] in
guard let self = self,
let textStorage = self.textStorage else {
return
}

// Verify the content hasn't changed while we were processing
guard contentHash == textStorage.string.hash else {
return
}

// Verify ranges are still valid
guard fullRange.upperBound <= textStorage.length else {
return
}

// Disable the Delegate temporarily
let theDelegate = self.delegate
self.delegate = nil

/// Issue #472: Linkification should not be undoable
self.undoManager?.disableUndoRegistration()

if let storage = textStorage as? Storage {
storage.beginEditing()

// Remove existing link attributes
textStorage.removeAttribute(.link, range: fullRange)

// Add link attributes while preserving existing ones
for match in matches where match.resultType == .link {
// Verify each match range is still valid
guard match.range.upperBound <= textStorage.length else {
continue
}

if let url = match.url {
textStorage.addAttribute(.link, value: url, range: match.range)
} else {
// If no URL, create one from the matched text
let text = textStorage.string as NSString
let linkText = text.substring(with: match.range)
if let url = URL(string: linkText) {
textStorage.addAttribute(.link, value: url, range: match.range)
}
}
}

storage.endEditingWithoutRestyling()
}

self.undoManager?.enableUndoRegistration()

/// Restore the Delegate
self.delegate = theDelegate
}
}
}
}

Expand Down
2 changes: 0 additions & 2 deletions Simplenote/NoteEditorViewController+Swift.swift
Original file line number Diff line number Diff line change
Expand Up @@ -962,8 +962,6 @@ extension NoteEditorViewController {
scrollView.scrollToTop(animated: false)
return
}
// ensure layout to make sure that content size is updated
noteEditor.ensureLayout()
scrollView.documentView?.scroll(NSPoint(x: 0, y: scrollPosition))
}

Expand Down
1 change: 0 additions & 1 deletion Simplenote/SPTextView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ extension SPTextView {
return 0.0
}

ensureLayout()
let textContainerHeight = layoutManager.usedRect(for: textContainer).size.height
let textContainerMinHeight = scrollView.frame.size.height - scrollView.scrollerInsets.top
return max(textContainerHeight, textContainerMinHeight)
Expand Down
Loading