Skip to content

Commit 7ff8f39

Browse files
authored
[shared] Fix Find highlights going stale after edits (#264) (#293)
* [shared] Fix stale find highlights after edits Clear find highlights on edit and surface a "stale" state in the find bar so the user knows to retype to refresh. ⌘G / Find Next re-runs the search when matches were cleared. Mac uses layout-manager temporary attributes so find backgrounds don't collide with `==highlight==` syntax; iOS keys removal on the find color value for the same reason. Also clears matches on external text replacement (file load/revert). * [shared] Fix find paint losing the right keystroke and ==highlight== bg Two bugs surfaced after the initial #264 fix landed. Mac's `@Published.$query` sink re-read `findState.query` when it fired, but `@Published` publishes in willSet — so the read returned the OLD value and every search ran one keystroke behind; pass `newQuery` from the sink directly into `performFind`. Both platforms also painted find color onto `NSTextStorage`, which overwrote any `==highlight==` markdown background it landed on; Mac now uses `NSLayoutManager.addTemporaryAttribute` (Apple's pattern for transient UI highlights, doesn't touch storage), and iOS re-runs the syntax highlighter on apply/clear since UIKit's `NSLayoutManager` has no temp-attribute API.
1 parent fc014ca commit 7ff8f39

8 files changed

Lines changed: 137 additions & 51 deletions

File tree

Clearly/EditorView.swift

Lines changed: 50 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ struct EditorView: NSViewRepresentable {
313313
context.coordinator.isHighlightingInProgress = true
314314
context.coordinator.highlighter?.highlightAll(textView.textStorage!, caller: "appearance")
315315
context.coordinator.isHighlightingInProgress = false
316-
context.coordinator.restoreFindHighlightsIfNeeded()
317316

318317
// Refresh ruler when appearance/font changes
319318
context.coordinator.gutterView?.appearanceDidChange()
@@ -334,7 +333,16 @@ struct EditorView: NSViewRepresentable {
334333
context.coordinator.isHighlightingInProgress = true
335334
context.coordinator.highlighter?.highlightAll(textView.textStorage!, caller: "externalText")
336335
context.coordinator.isHighlightingInProgress = false
337-
context.coordinator.restoreFindHighlightsIfNeeded()
336+
// External text replacement (file load/revert) — old match ranges are stale.
337+
context.coordinator.clearFindHighlights()
338+
if let findState = context.coordinator.findState, findState.isVisible, findState.activeMode == .edit {
339+
DispatchQueue.main.async { [weak findState] in
340+
guard let findState, findState.activeMode == .edit else { return }
341+
findState.matchCount = 0
342+
findState.currentIndex = 0
343+
findState.resultsAreStale = true
344+
}
345+
}
338346
context.coordinator.gutterView?.textDidChange()
339347
context.coordinator.gutterView?.selectionDidChange(selectedRange: textView.selectedRange())
340348
context.coordinator.isUpdating = false
@@ -521,12 +529,15 @@ struct EditorView: NSViewRepresentable {
521529

522530
state.$query
523531
.removeDuplicates()
524-
.sink { [weak self] _ in
532+
.sink { [weak self] newQuery in
525533
guard let self,
526534
let findState = self.findState,
527535
findState.isVisible,
528536
findState.activeMode == .edit else { return }
529-
self.performFind()
537+
// `@Published` fires in willSet — `findState.query` still
538+
// reads the OLD value here. Pass `newQuery` explicitly so
539+
// performFind doesn't run a keystroke behind.
540+
self.performFind(query: newQuery)
530541
}
531542
.store(in: &findCancellables)
532543

@@ -602,7 +613,6 @@ struct EditorView: NSViewRepresentable {
602613
sv.reflectScrolledClipView(sv.contentView)
603614
}
604615
self.invalidateVisibleRegion(of: textView)
605-
self.restoreFindHighlightsIfNeeded()
606616
}
607617
pendingFullHighlightWork = work
608618
DispatchQueue.main.asyncAfter(deadline: .now() + 0.3, execute: work)
@@ -619,8 +629,18 @@ struct EditorView: NSViewRepresentable {
619629
// the visible rect so large documents don't repaint end-to-end.
620630
invalidateVisibleRegion(of: textView)
621631

622-
// Re-apply find highlights after syntax highlighting
623-
restoreFindHighlightsIfNeeded()
632+
// Clear on edit; user retypes the query to refresh (#264).
633+
if let findState, findState.isVisible, !matchRanges.isEmpty {
634+
clearFindHighlights()
635+
if findState.activeMode == .edit {
636+
DispatchQueue.main.async { [weak findState] in
637+
guard let findState, findState.activeMode == .edit else { return }
638+
findState.matchCount = 0
639+
findState.currentIndex = 0
640+
findState.resultsAreStale = true
641+
}
642+
}
643+
}
624644

625645
// Update line number ruler
626646
gutterView?.textDidChange()
@@ -763,17 +783,9 @@ struct EditorView: NSViewRepresentable {
763783

764784
// MARK: - Find
765785

766-
func restoreFindHighlightsIfNeeded() {
767-
guard findState?.isVisible == true, !(findState?.query.isEmpty ?? true) else { return }
768-
// Re-apply cached highlight colors without re-running the full find
769-
// (which would scroll to the first match on every keystroke).
770-
guard !matchRanges.isEmpty else { return }
771-
applyFindHighlights()
772-
}
773-
774-
func performFind() {
786+
func performFind(query overrideQuery: String? = nil) {
775787
guard let textView, let findState else { return }
776-
let query = findState.query
788+
let query = overrideQuery ?? findState.query
777789
clearFindHighlights()
778790

779791
guard !query.isEmpty else {
@@ -785,6 +797,7 @@ struct EditorView: NSViewRepresentable {
785797
findState.activeMode == .edit else { return }
786798
findState.matchCount = 0
787799
findState.currentIndex = 0
800+
findState.resultsAreStale = false
788801
}
789802
return
790803
}
@@ -801,6 +814,7 @@ struct EditorView: NSViewRepresentable {
801814
findState.activeMode == .edit else { return }
802815
findState.matchCount = ranges.count
803816
findState.currentIndex = ranges.isEmpty ? 0 : 1
817+
findState.resultsAreStale = false
804818
}
805819

806820
if !ranges.isEmpty {
@@ -809,6 +823,12 @@ struct EditorView: NSViewRepresentable {
809823
}
810824

811825
func navigateToNextMatch() {
826+
// After clear-on-edit, matchRanges is empty but the user's query is
827+
// still in the find bar — treat ⌘G / Find Next as a re-run trigger.
828+
if matchRanges.isEmpty, let findState, !findState.query.isEmpty {
829+
performFind()
830+
return
831+
}
812832
guard !matchRanges.isEmpty else { return }
813833
currentMatchIdx = (currentMatchIdx + 1) % matchRanges.count
814834
applyFindHighlights()
@@ -822,6 +842,10 @@ struct EditorView: NSViewRepresentable {
822842
}
823843

824844
func navigateToPreviousMatch() {
845+
if matchRanges.isEmpty, let findState, !findState.query.isEmpty {
846+
performFind()
847+
return
848+
}
825849
guard !matchRanges.isEmpty else { return }
826850
currentMatchIdx = (currentMatchIdx - 1 + matchRanges.count) % matchRanges.count
827851
applyFindHighlights()
@@ -834,31 +858,27 @@ struct EditorView: NSViewRepresentable {
834858
}
835859
}
836860

861+
// Find highlights live on the layout manager via temporary attributes,
862+
// not on text storage. This is Apple's recommended pattern for
863+
// transient UI highlighting and means find painting never overwrites
864+
// `==highlight==` markdown backgrounds (which DO live on storage).
837865
private func applyFindHighlights() {
838-
guard let textView else { return }
866+
guard let textView, let layoutManager = textView.layoutManager else { return }
839867
let storage = textView.textStorage!
840-
841-
// Clear existing find highlights first
842868
let fullRange = NSRange(location: 0, length: storage.length)
843-
storage.beginEditing()
844-
storage.removeAttribute(.backgroundColor, range: fullRange)
845-
846-
// Apply highlight to all matches
869+
layoutManager.removeTemporaryAttribute(.backgroundColor, forCharacterRange: fullRange)
847870
for (i, range) in matchRanges.enumerated() {
848871
guard range.upperBound <= storage.length else { continue }
849872
let color = (i == currentMatchIdx) ? Theme.findCurrentHighlightColor : Theme.findHighlightColor
850-
storage.addAttribute(.backgroundColor, value: color, range: range)
873+
layoutManager.addTemporaryAttribute(.backgroundColor, value: color, forCharacterRange: range)
851874
}
852-
storage.endEditing()
853875
}
854876

855877
func clearFindHighlights() {
856-
guard let textView else { return }
878+
guard let textView, let layoutManager = textView.layoutManager else { return }
857879
let storage = textView.textStorage!
858880
let fullRange = NSRange(location: 0, length: storage.length)
859-
storage.beginEditing()
860-
storage.removeAttribute(.backgroundColor, range: fullRange)
861-
storage.endEditing()
881+
layoutManager.removeTemporaryAttribute(.backgroundColor, forCharacterRange: fullRange)
862882
matchRanges = []
863883
currentMatchIdx = 0
864884
}

Clearly/FindBarView.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ struct FindBarView: View {
2323
)
2424
.frame(minWidth: 120)
2525

26-
if !findState.query.isEmpty {
26+
if !findState.query.isEmpty && !findState.resultsAreStale {
2727
if findState.matchCount > 0 {
2828
Text("\(findState.currentIndex) of \(findState.matchCount)")
2929
.font(.system(size: 11, weight: .medium))
@@ -49,10 +49,10 @@ struct FindBarView: View {
4949
)
5050

5151
HStack(spacing: 2) {
52-
FindNavButton(icon: "chevron.left", disabled: findState.matchCount == 0) {
52+
FindNavButton(icon: "chevron.left", disabled: !findState.canNavigate) {
5353
findState.navigateToPrevious?()
5454
}
55-
FindNavButton(icon: "chevron.right", disabled: findState.matchCount == 0) {
55+
FindNavButton(icon: "chevron.right", disabled: !findState.canNavigate) {
5656
findState.navigateToNext?()
5757
}
5858
}

Clearly/LiveEditorView.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,7 @@ struct LiveEditorView: NSViewRepresentable {
429429
guard self.parent.findState?.activeMode == .edit || self.parent.findState?.isVisible == false else { return }
430430
self.parent.findState?.matchCount = matchCount
431431
self.parent.findState?.currentIndex = currentIndex
432+
self.parent.findState?.resultsAreStale = false
432433
}
433434

434435
case "openLink":

Clearly/PreviewView.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,7 @@ struct PreviewView: NSViewRepresentable {
644644
guard self.findState?.activeMode == .preview else { return }
645645
self.findState?.matchCount = count
646646
self.findState?.currentIndex = count > 0 ? 1 : 0
647+
self.findState?.resultsAreStale = false
647648
}
648649
}
649650
}
@@ -695,6 +696,7 @@ struct PreviewView: NSViewRepresentable {
695696
guard self?.findState?.activeMode == .preview || self?.findState?.isVisible == false else { return }
696697
self?.findState?.matchCount = 0
697698
self?.findState?.currentIndex = 0
699+
self?.findState?.resultsAreStale = false
698700
}
699701
}
700702

Clearly/iOS/EditorView_iOS.swift

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ struct EditorView_iOS: UIViewRepresentable {
128128
guard let state, state.activeMode == .edit else { return }
129129
state.matchCount = ranges.count
130130
state.currentIndex = ranges.isEmpty ? 0 : 1
131+
state.resultsAreStale = false
131132
_ = self
132133
}
133134

@@ -137,6 +138,12 @@ struct EditorView_iOS: UIViewRepresentable {
137138
}
138139

139140
private func navigateToNextMatch() {
141+
// After clear-on-edit, matchRanges is empty but the query is still
142+
// in the find bar — treat Next as a re-run trigger.
143+
if matchRanges.isEmpty, let state = attachedFindState, !state.query.isEmpty {
144+
performFind(for: state)
145+
return
146+
}
140147
guard !matchRanges.isEmpty else { return }
141148
currentMatchIdx = (currentMatchIdx + 1) % matchRanges.count
142149
applyFindHighlights()
@@ -149,6 +156,10 @@ struct EditorView_iOS: UIViewRepresentable {
149156
}
150157

151158
private func navigateToPreviousMatch() {
159+
if matchRanges.isEmpty, let state = attachedFindState, !state.query.isEmpty {
160+
performFind(for: state)
161+
return
162+
}
152163
guard !matchRanges.isEmpty else { return }
153164
currentMatchIdx = (currentMatchIdx - 1 + matchRanges.count) % matchRanges.count
154165
applyFindHighlights()
@@ -160,12 +171,17 @@ struct EditorView_iOS: UIViewRepresentable {
160171
}
161172
}
162173

174+
// UIKit's NSLayoutManager has no temporary-attribute API, so find
175+
// highlights have to use storage attributes. To prevent find color
176+
// from clobbering `==highlight==` markdown backgrounds, we re-run
177+
// the syntax highlighter (which resets bg per paragraph) before
178+
// every paint — and again on clear so dismissed find never leaves
179+
// missing backgrounds behind.
163180
private func applyFindHighlights() {
164181
guard let textView else { return }
165182
let storage = textView.textStorage
166-
let fullRange = NSRange(location: 0, length: storage.length)
183+
rerunSyntaxHighlighter(on: storage)
167184
storage.beginEditing()
168-
storage.removeAttribute(.backgroundColor, range: fullRange)
169185
for (i, range) in matchRanges.enumerated() {
170186
guard range.upperBound <= storage.length else { continue }
171187
let color = (i == currentMatchIdx) ? Theme.findCurrentHighlightColor : Theme.findHighlightColor
@@ -176,22 +192,16 @@ struct EditorView_iOS: UIViewRepresentable {
176192

177193
private func clearFindHighlights() {
178194
guard let textView else { return }
179-
let storage = textView.textStorage
180-
let fullRange = NSRange(location: 0, length: storage.length)
181-
storage.beginEditing()
182-
storage.removeAttribute(.backgroundColor, range: fullRange)
183-
storage.endEditing()
195+
rerunSyntaxHighlighter(on: textView.textStorage)
184196
matchRanges = []
185197
currentMatchIdx = 0
186198
}
187199

188-
/// Called at the tail of `textViewDidChange` so the syntax-highlighter's
189-
/// attribute rewrite doesn't wipe find backgrounds. Mirrors the Mac
190-
/// `restoreFindHighlightsIfNeeded` pattern — if the query is still
191-
/// active, re-run the search so match ranges track the edit too.
192-
private func restoreFindHighlightsIfNeeded() {
193-
guard let state = attachedFindState, state.isVisible, !state.query.isEmpty else { return }
194-
performFind(for: state)
200+
private func rerunSyntaxHighlighter(on storage: NSTextStorage) {
201+
let wasHighlighting = isHighlighting
202+
isHighlighting = true
203+
highlighter.highlightAll(storage, caller: "find-rerun")
204+
isHighlighting = wasHighlighting
195205
}
196206

197207
private func scrollToRange(_ range: NSRange) {
@@ -221,6 +231,20 @@ struct EditorView_iOS: UIViewRepresentable {
221231
textView.selectedRange = clamped
222232
isHighlighting = false
223233
lastAppliedText = newText
234+
235+
// External text replacement (file load/revert) — old match ranges are stale.
236+
if !matchRanges.isEmpty {
237+
matchRanges = []
238+
currentMatchIdx = 0
239+
if let state = attachedFindState, state.isVisible, state.activeMode == .edit {
240+
DispatchQueue.main.async { [weak state] in
241+
guard let state, state.activeMode == .edit else { return }
242+
state.matchCount = 0
243+
state.currentIndex = 0
244+
state.resultsAreStale = true
245+
}
246+
}
247+
}
224248
}
225249

226250
// MARK: - UITextViewDelegate
@@ -272,7 +296,18 @@ struct EditorView_iOS: UIViewRepresentable {
272296
lastAppliedText = newText
273297
parent.text = newText
274298

275-
restoreFindHighlightsIfNeeded()
299+
// Clear on edit; user retypes the query to refresh (#264).
300+
if let state = attachedFindState, state.isVisible, !matchRanges.isEmpty {
301+
clearFindHighlights()
302+
if state.activeMode == .edit {
303+
DispatchQueue.main.async { [weak state] in
304+
guard let state, state.activeMode == .edit else { return }
305+
state.matchCount = 0
306+
state.currentIndex = 0
307+
state.resultsAreStale = true
308+
}
309+
}
310+
}
276311

277312
let token = UUID()
278313
pendingBindingUpdateToken = token

Clearly/iOS/FindOverlay_iOS.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ struct FindOverlay_iOS: View {
2323
.focused($isFieldFocused)
2424
.onSubmit { findState.navigateToNext?() }
2525

26-
if !findState.query.isEmpty {
26+
if !findState.query.isEmpty && !findState.resultsAreStale {
2727
if findState.matchCount > 0 {
2828
Text("\(findState.currentIndex) of \(findState.matchCount)")
2929
.font(Theme.Typography.findCount)
@@ -62,10 +62,10 @@ struct FindOverlay_iOS: View {
6262
)
6363

6464
HStack(spacing: 2) {
65-
FindNavButton_iOS(icon: "chevron.up", disabled: findState.matchCount == 0) {
65+
FindNavButton_iOS(icon: "chevron.up", disabled: !findState.canNavigate) {
6666
findState.navigateToPrevious?()
6767
}
68-
FindNavButton_iOS(icon: "chevron.down", disabled: findState.matchCount == 0) {
68+
FindNavButton_iOS(icon: "chevron.down", disabled: !findState.canNavigate) {
6969
findState.navigateToNext?()
7070
}
7171
}

Packages/ClearlyCore/Sources/ClearlyCore/State/FindState.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ public final class FindState: ObservableObject {
77
@Published public var query = ""
88
@Published public var matchCount = 0
99
@Published public var currentIndex = 0 // 1-based, 0 = no matches
10+
@Published public var resultsAreStale = false
1011
@Published public var focusRequest = UUID()
1112
public var activeMode: ViewMode = .edit
1213

@@ -15,6 +16,10 @@ public final class FindState: ObservableObject {
1516
public var previewNavigateToNext: (() -> Void)?
1617
public var previewNavigateToPrevious: (() -> Void)?
1718

19+
public var canNavigate: Bool {
20+
!query.isEmpty && (resultsAreStale || matchCount > 0)
21+
}
22+
1823
public var navigateToNext: (() -> Void)? {
1924
switch activeMode {
2025
case .edit:

0 commit comments

Comments
 (0)