mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-08-29 07:27:16 +00:00
refactor(formatting): share the overlap-resolution pass between formatters
Per review: MessageFormattingEngine and ChatMessageFormatter each grew their own copy of the sort-then-drop-overlaps pass, which risks drifting apart later. Extract it into a single generic helper, MessageFormattingEngine.resolveOverlappingMatches, keyed by an NSRange extractor closure so it works over both formatters' different match representations (a ContentMatch struct vs. a (range, type) tuple) without unifying those types. Add direct unit tests for the extracted helper (nested overlap dropped, out-of-order input sorted internally, adjacent-but-non-overlapping ranges both kept) since ChatMessageFormatter's copy wasn't independently unit-testable without a live ChatViewModel.
This commit is contained in:
parent
7ba358ce46
commit
159f2bca83
@ -368,23 +368,35 @@ final class MessageFormattingEngine {
|
||||
allMatches.append(ContentMatch(range: match.range(at: 0), type: .lnurl))
|
||||
}
|
||||
|
||||
// Sort by position, then drop any match that overlaps one already
|
||||
// kept. The per-type checks above only guard specific known pairs
|
||||
// (e.g. bolt11/lnurl against url/lightning) -- they don't cover every
|
||||
// The per-type checks above only guard specific known pairs (e.g.
|
||||
// bolt11/lnurl against url/lightning) -- they don't cover every
|
||||
// combination, so an ordinary message can still produce overlapping
|
||||
// matches of two OTHER types the checks never compared (e.g. a URL
|
||||
// whose path embeds a "cashuA..."-shaped token). formatContent's
|
||||
// single linear pass assumes non-overlapping, strictly-increasing
|
||||
// ranges; without this pass a nested match re-renders already-shown
|
||||
// text and can walk `lastEnd` backwards, duplicating a trailing
|
||||
// slice of content a second time.
|
||||
let sorted = allMatches.sorted { $0.range.location < $1.range.location }
|
||||
var resolved: [ContentMatch] = []
|
||||
// whose path embeds a "cashuA..."-shaped token). Resolve any
|
||||
// remaining overlap before handing matches to a rendering pass.
|
||||
return resolveOverlappingMatches(allMatches) { $0.range }
|
||||
}
|
||||
|
||||
/// Sorts matches by start position and greedily keeps only non-overlapping
|
||||
/// ones, dropping any later match whose start falls inside an
|
||||
/// already-kept match's range. Rendering passes that consume the result
|
||||
/// (here and in `ChatMessageFormatter`, which has its own copy of the
|
||||
/// match-collection logic above) assume non-overlapping,
|
||||
/// strictly-increasing ranges; without this resolution a nested match
|
||||
/// re-renders already-shown text and can walk the render cursor
|
||||
/// backwards, duplicating a trailing slice of content a second time.
|
||||
static func resolveOverlappingMatches<Match>(
|
||||
_ matches: [Match],
|
||||
range: (Match) -> NSRange
|
||||
) -> [Match] {
|
||||
let sorted = matches.sorted { range($0).location < range($1).location }
|
||||
var resolved: [Match] = []
|
||||
var occupiedUntil = 0
|
||||
for match in sorted {
|
||||
guard match.range.location >= occupiedUntil else { continue }
|
||||
let matchRange = range(match)
|
||||
guard matchRange.location >= occupiedUntil else { continue }
|
||||
resolved.append(match)
|
||||
occupiedUntil = match.range.location + match.range.length
|
||||
occupiedUntil = matchRange.location + matchRange.length
|
||||
}
|
||||
return resolved
|
||||
}
|
||||
|
||||
@ -188,25 +188,17 @@ final class ChatMessageFormatter {
|
||||
where !overlapsMention(match.range(at: 0)) && !overlapsOccupied(match.range(at: 0)) {
|
||||
allMatches.append((match.range(at: 0), "lnurl"))
|
||||
}
|
||||
allMatches.sort { $0.range.location < $1.range.location }
|
||||
|
||||
// Drop any match that overlaps one already kept. The
|
||||
// per-type checks above only guard specific known pairs
|
||||
// The per-type checks above only guard specific known pairs
|
||||
// (e.g. bolt11/lnurl against url+lightning); they don't
|
||||
// cover every combination, so an ordinary message can still
|
||||
// produce overlapping matches of two OTHER types (e.g. a URL
|
||||
// whose path embeds a "cashuA..."-shaped token). The render
|
||||
// loop below assumes non-overlapping, strictly-increasing
|
||||
// ranges; without this pass a nested cashu/lightning match
|
||||
// renders as an extra spacer character injected into
|
||||
// already-shown text instead of being skipped.
|
||||
var resolvedMatches: [(range: NSRange, type: String)] = []
|
||||
var occupiedUntil = 0
|
||||
for match in allMatches {
|
||||
guard match.range.location >= occupiedUntil else { continue }
|
||||
resolvedMatches.append(match)
|
||||
occupiedUntil = match.range.location + match.range.length
|
||||
}
|
||||
// ranges; without resolving those first, a nested
|
||||
// cashu/lightning match renders as an extra spacer character
|
||||
// injected into already-shown text instead of being skipped.
|
||||
let resolvedMatches = MessageFormattingEngine.resolveOverlappingMatches(allMatches) { $0.range }
|
||||
|
||||
var lastEnd = content.startIndex
|
||||
let myNickname = viewModel.nickname.normalizedNickname
|
||||
|
||||
@ -375,6 +375,43 @@ struct MessageFormattingEngineTests {
|
||||
|
||||
#expect(String(formatted.characters) == "<@alice> \(content) [\(message.formattedTimestamp)]")
|
||||
}
|
||||
|
||||
// MARK: - resolveOverlappingMatches Tests
|
||||
//
|
||||
// Direct coverage for the shared helper `ChatMessageFormatter` also
|
||||
// calls, since its own match-collection logic (a hand-copy of
|
||||
// findAllMatches above) isn't independently unit-testable without a
|
||||
// live ChatViewModel.
|
||||
|
||||
@Test func resolveOverlappingMatches_dropsANestedOverlap() {
|
||||
let outer = (range: NSRange(location: 0, length: 10), type: "outer")
|
||||
let nested = (range: NSRange(location: 3, length: 2), type: "nested")
|
||||
|
||||
let resolved = MessageFormattingEngine.resolveOverlappingMatches([outer, nested]) { $0.range }
|
||||
|
||||
#expect(resolved.map(\.type) == ["outer"])
|
||||
}
|
||||
|
||||
@Test func resolveOverlappingMatches_keepsNonOverlappingMatchesSortedByStart() {
|
||||
let first = (range: NSRange(location: 0, length: 3), type: "first")
|
||||
let second = (range: NSRange(location: 5, length: 3), type: "second")
|
||||
|
||||
// Passed out of order; the helper must sort internally.
|
||||
let resolved = MessageFormattingEngine.resolveOverlappingMatches([second, first]) { $0.range }
|
||||
|
||||
#expect(resolved.map(\.type) == ["first", "second"])
|
||||
}
|
||||
|
||||
@Test func resolveOverlappingMatches_adjacentNonOverlappingMatchesAreBothKept() {
|
||||
// A match starting exactly where the previous one ends is not an
|
||||
// overlap (NSRange upper bound is exclusive).
|
||||
let first = (range: NSRange(location: 0, length: 5), type: "first")
|
||||
let adjacent = (range: NSRange(location: 5, length: 5), type: "adjacent")
|
||||
|
||||
let resolved = MessageFormattingEngine.resolveOverlappingMatches([first, adjacent]) { $0.range }
|
||||
|
||||
#expect(resolved.map(\.type) == ["first", "adjacent"])
|
||||
}
|
||||
}
|
||||
|
||||
@MainActor
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user