diff --git a/Sources/Textual/Internal/StructuredText/Table.swift b/Sources/Textual/Internal/StructuredText/Table.swift index b3fdd8b0..f39c7a28 100644 --- a/Sources/Textual/Internal/StructuredText/Table.swift +++ b/Sources/Textual/Internal/StructuredText/Table.swift @@ -49,11 +49,16 @@ extension StructuredText { let rowRun = rowRuns[rowIndex] let rowContent = content[rowRun.range] let columnRuns = rowContent.blockRuns(parent: rowRun.intent) + // Foundation omits a run for any empty cell, so iterating `columnRuns` + // positionally would shift every cell after the gap one column left + // (an empty leading header collapses onto the wrong columns). Instead + // lay each cell out at the column ordinal carried by its `tableCell` + // intent, leaving an empty cell where a run is missing. + let cellRanges = Self.cellRanges(columnRuns, declaredColumns: columns.count) GridRow { - ForEach(columnRuns.indices, id: \.self) { columnIndex in - let cellRun = columnRuns[columnIndex] - let cellContent = rowContent[cellRun.range] + ForEach(cellRanges.indices, id: \.self) { columnIndex in + let cellContent = cellRanges[columnIndex].map { rowContent[$0] } TableCell(cellContent, row: rowIndex, column: columnIndex) .gridColumnAlignment(alignment(for: columnIndex)) @@ -63,6 +68,46 @@ extension StructuredText { } } + /// Maps a row's block runs to a dense, column-indexed array of cell ranges. + /// + /// Foundation tags every table cell with its ordinal via + /// `PresentationIntent.Kind.tableCell(columnIndex:)` but emits no run at all + /// for a cell with no text. Placing runs by that ordinal — rather than by + /// their position in `columnRuns` — keeps columns aligned across rows even + /// when a cell is empty. Slots with no run are `nil` and render blank. + /// + /// If any run lacks an ordinal (defensive: malformed/unexpected input) we + /// fall back to the original positional layout for that row. + private static func cellRanges( + _ columnRuns: AttributedString.BlockRuns, + declaredColumns: Int + ) -> [Range?] { + let ordinals = columnRuns.indices.map { columnOrdinal(of: columnRuns[$0]) } + + guard ordinals.allSatisfy({ $0 != nil }) else { + return columnRuns.indices.map { columnRuns[$0].range } + } + + let maxOrdinal = ordinals.compactMap { $0 }.max() ?? -1 + let width = max(declaredColumns, maxOrdinal + 1) + var ranges = [Range?](repeating: nil, count: width) + for runIndex in columnRuns.indices { + if let column = ordinals[runIndex], column < width { + ranges[column] = columnRuns[runIndex].range + } + } + return ranges + } + + private static func columnOrdinal( + of run: AttributedString.BlockRuns.BlockRun + ) -> Int? { + guard case .tableCell(let columnIndex)? = run.intent?.kind else { + return nil + } + return columnIndex + } + private var indentationLevel: Int { content.runs.first?.presentationIntent?.indentationLevel ?? 0 } diff --git a/Sources/Textual/Internal/StructuredText/TableCell.swift b/Sources/Textual/Internal/StructuredText/TableCell.swift index 6e402f27..1a384db3 100644 --- a/Sources/Textual/Internal/StructuredText/TableCell.swift +++ b/Sources/Textual/Internal/StructuredText/TableCell.swift @@ -4,10 +4,13 @@ extension StructuredText { struct TableCell: View { @Environment(\.tableCellStyle) private var tableCellStyle - private let content: AttributedSubstring + private let content: AttributedSubstring? private let identifier: TableCell.Identifier - init(_ content: AttributedSubstring, row: Int, column: Int) { + /// `content` is optional because Foundation's markdown parser omits a run for + /// any cell that has no text (e.g. an empty leading header cell). Such cells + /// must still render so the column stays aligned, hence a `nil` content slot. + init(_ content: AttributedSubstring?, row: Int, column: Int) { self.content = content self.identifier = .init(row: row, column: column) } @@ -30,13 +33,13 @@ extension StructuredText { } private var label: some View { - WithInlineStyle(AttributedString(content)) { + WithInlineStyle(content.map(AttributedString.init) ?? AttributedString()) { TextFragment($0) } } private var indentationLevel: Int { - content.presentationIntent?.indentationLevel ?? 0 + content?.presentationIntent?.indentationLevel ?? 0 } } } diff --git a/Sources/Textual/Internal/TextFragment/TextBuilder.swift b/Sources/Textual/Internal/TextFragment/TextBuilder.swift index 1910ba2d..7569fb6d 100644 --- a/Sources/Textual/Internal/TextFragment/TextBuilder.swift +++ b/Sources/Textual/Internal/TextFragment/TextBuilder.swift @@ -15,6 +15,30 @@ import SwiftUI // Runs with attachments are converted to placeholder images sized by the attachment's // sizeThatFits(_:in:) result. Placeholders are tagged with AttachmentAttribute so overlays // can identify and render the actual attachment views at the resolved layout positions. +// +// MARK: - Recursion-safe Text construction +// +// Only attachment placeholders and link runs need to be standalone Text nodes — they +// rely on `customAttribute(AttachmentAttribute(...))` and `customAttribute(LinkAttribute(...))` +// markers that cannot live inside an AttributedString. Every other run carries standard +// AttributedString attributes (foregroundColor, font, presentationIntent, syntax-highlight +// theme tokens, etc.) which Text(_ attributedString:) preserves natively. +// +// Building one Text per run and then `Text("\(prev)\(next)")`-reducing them into a single +// value produces a LocalizedTextStorage tree whose depth equals the run count. SwiftUI +// resolves that tree recursively at layout time, so a code block with thousands of Prism +// tokens (or any AttributedString with thousands of runs) blows the main-thread stack at +// resolve time — the "Thread stack size exceeded due to excessive recursion" crash. +// +// Instead, consecutive plain runs are coalesced into a single Text(_ attributedString:) +// and concatenated with the `+` operator only across attachment / link boundaries. The +// resulting Text-node count is bounded by `attachments + links + 1` rather than the run +// count, and the concatenation uses ConcatenatedTextStorage instead of LocalizedTextStorage, +// which avoids the localization-machinery walk entirely. +// +// The final `+`-merge is balanced (pairwise) so the ConcatenatedTextStorage tree is +// O(log N) deep, not O(N). A left-fold reduce would re-introduce the same crash class +// in a different guise for content with thousands of attachments / links. extension TextFragment { @MainActor @Observable final class TextBuilder { @@ -64,23 +88,43 @@ extension Text { attachmentSizes: [AttachmentKey: CGSize], in environment: TextEnvironmentValues ) { - let textValues = attributedString.runs.map { run in - var text: Text + var pieces: [Text] = [] + var pending = AttributedString() - var runEnvironment = environment - runEnvironment.font = run.font ?? environment.font + func flushPending() { + guard !pending.runs.isEmpty else { return } + pieces.append(Text(pending)) + pending = AttributedString() + } - let key = run.textual.attachment.map { - AttachmentKey(attachment: $0, font: runEnvironment.font) + for run in attributedString.runs { + let attachment = run.textual.attachment + let link = run.link + + // Plain runs (no attachment, no link) carry only AttributedString attributes, + // so they round-trip cleanly through Text(_ attributedString:). Coalesce them + // into `pending` and only flush at attachment / link boundaries. The 99% case + // (every Prism token in a code block, every styled span in prose) lands here + // and never builds a runEnvironment or a separate Text node. + if attachment == nil, link == nil { + pending.append(AttributedString(attributedString[run.range])) + continue } - if let key, let size = attachmentSizes[key] { - // Create placeholder + flushPending() + + var runEnvironment = environment + runEnvironment.font = run.font ?? environment.font + + var text: Text + if let attachment, + let size = attachmentSizes[AttachmentKey(attachment: attachment, font: runEnvironment.font)] + { text = Text(placeholderSize: size) - .baselineOffset(key.attachment.baselineOffset(in: runEnvironment)) + .baselineOffset(attachment.baselineOffset(in: runEnvironment)) .customAttribute( AttachmentAttribute( - key.attachment, + attachment, presentationIntent: run.presentationIntent ) ) @@ -88,17 +132,43 @@ extension Text { text = Text(AttributedString(attributedString[run.range])) } - // Add link attribute for TextLinkInteraction - if let link = run.link { + if let link { text = text.customAttribute(LinkAttribute(link)) } - return text + pieces.append(text) } - self = textValues.reduce(Text(verbatim: "")) { partialResult, text in - Text("\(partialResult)\(text)") + flushPending() + + self = Text.balancedConcatenation(of: pieces) + } + + /// Pairwise merge of `pieces` using `+`, producing a `ConcatenatedTextStorage` + /// tree of O(log N) depth instead of the O(N) depth a left-fold would yield. + /// `+`-resolve still recurses, so a balanced tree is the difference between + /// safely rendering thousands of inline pieces and overflowing the stack at + /// layout time. The construction cost stays O(N). + /// + /// Internal (rather than fileprivate) so unit tests can exercise the helper + /// directly without going through full `TextBuilder` construction. + static func balancedConcatenation(of pieces: [Text]) -> Text { + guard !pieces.isEmpty else { return Text(verbatim: "") } + var level = pieces + while level.count > 1 { + var next: [Text] = [] + next.reserveCapacity((level.count + 1) / 2) + var index = 0 + while index + 1 < level.count { + next.append(level[index] + level[index + 1]) + index += 2 + } + if index < level.count { + next.append(level[index]) + } + level = next } + return level[0] } private init(placeholderSize size: CGSize) { diff --git a/Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextSelectionInteraction.swift b/Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextSelectionInteraction.swift index c3b571c9..7b05bbe7 100644 --- a/Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextSelectionInteraction.swift +++ b/Sources/Textual/Internal/TextInteraction/AppKit/AppKitTextSelectionInteraction.swift @@ -26,6 +26,11 @@ // We need the selection model at text fragment level for the // text selection background and selected attachment dimming .environment(model) + // Resolve geometry before distributing to the NSView overlay. Without + // this, animated container resizes (e.g. NavigationSplitView sidebar + // collapse) can leave the NSTextInteractionView with a stale frame, + // causing it to intercept mouse events outside its visible bounds. + .geometryGroup() .overlayPreferenceValue(OverflowFrameKey.self) { frames in AppKitTextInteractionOverlay(model: model, overflowFrames: frames) .onContinuousHover { phase in diff --git a/Sources/Textual/TextualNamespace.swift b/Sources/Textual/TextualNamespace.swift index ea74b0f4..6fde4927 100644 --- a/Sources/Textual/TextualNamespace.swift +++ b/Sources/Textual/TextualNamespace.swift @@ -18,7 +18,7 @@ import Foundation /// SwiftUI views get the namespace through the ``SwiftUICore/View/textual`` property. /// Other types can opt into it by conforming to ``TextualCompatible``. public struct TextualNamespace { - @usableFromInline let base: Base + @usableFromInline var base: Base @inlinable public init(_ base: Base) { self.base = base } } diff --git a/Tests/TextualTests/Internal/TextFragment/TextBuilderTests.swift b/Tests/TextualTests/Internal/TextFragment/TextBuilderTests.swift new file mode 100644 index 00000000..f022fc97 --- /dev/null +++ b/Tests/TextualTests/Internal/TextFragment/TextBuilderTests.swift @@ -0,0 +1,118 @@ +import SwiftUI +import Testing + +@testable import Textual + +@MainActor +extension TextFragment { + struct TextBuilderTests { + private static func makeAttributedString( + runCount: Int, + colors: [Color] = [.red, .blue, .green, .orange, .purple] + ) -> AttributedString { + var result = AttributedString() + for index in 0 ..< runCount { + var piece = AttributedString("token-\(index) ") + piece.foregroundColor = colors[index % colors.count] + result.append(piece) + } + return result + } + + @Test func empty() { + let builder = TextFragment.TextBuilder( + AttributedString(), + environment: TextEnvironmentValues() + ) + #expect(type(of: builder.text) == Text.self) + } + + @Test func plainSingleRun() { + let builder = TextFragment.TextBuilder( + AttributedString("hello world"), + environment: TextEnvironmentValues() + ) + #expect(type(of: builder.text) == Text.self) + } + + /// A pathologically large run count that would overflow the SwiftUI layout stack + /// under the previous `Text("\(prev)\(next)")` reduction. We can't probe the + /// resulting Text-tree depth from outside SwiftUI, but we can at least confirm that + /// construction itself remains O(N) and doesn't recurse N levels — when it does, + /// it shows up as `EXC_BAD_ACCESS` here, not a passing test. + @Test func largeRunCountConstructsCleanly() { + let attributed = Self.makeAttributedString(runCount: 5000) + let builder = TextFragment.TextBuilder( + attributed, + environment: TextEnvironmentValues() + ) + #expect(type(of: builder.text) == Text.self) + } + + @Test func plainRunsAreCoalescedAcrossSizeChanges() { + let attributed = Self.makeAttributedString(runCount: 100) + let builder = TextFragment.TextBuilder( + attributed, + environment: TextEnvironmentValues() + ) + builder.sizeChanged(CGSize(width: 320, height: 480), environment: TextEnvironmentValues()) + builder.sizeChanged(CGSize(width: 480, height: 320), environment: TextEnvironmentValues()) + #expect(type(of: builder.text) == Text.self) + } + + @Test func runsWithLinks() { + var attributed = AttributedString("Visit ") + var linkRun = AttributedString("the example site") + linkRun.link = URL(string: "https://example.com") + attributed.append(linkRun) + attributed.append(AttributedString(" for details. ")) + + var trailingLink = AttributedString("Or here") + trailingLink.link = URL(string: "https://example.org") + attributed.append(trailingLink) + attributed.append(AttributedString(".")) + + let builder = TextFragment.TextBuilder( + attributed, + environment: TextEnvironmentValues() + ) + #expect(type(of: builder.text) == Text.self) + } + + /// Stress-tests the balanced concatenation path. Many link-bearing runs each force + /// a separate Text node; a left-fold concat would build a `ConcatenatedTextStorage` + /// tree of depth = link count and re-introduce the 2024-class recursion crash for + /// pathological link-list content. Pairwise merge keeps the tree O(log N) deep. + @Test func manyLinkRunsConstructCleanly() { + var attributed = AttributedString() + for index in 0 ..< 2000 { + var prose = AttributedString("see ") + prose.foregroundColor = .secondary + attributed.append(prose) + + var link = AttributedString("link-\(index)") + link.link = URL(string: "https://example.com/\(index)") + attributed.append(link) + + attributed.append(AttributedString(", ")) + } + + let builder = TextFragment.TextBuilder( + attributed, + environment: TextEnvironmentValues() + ) + #expect(type(of: builder.text) == Text.self) + } + + @Test func balancedConcatenationOfEmptyArrayIsEmpty() { + let result = Text.balancedConcatenation(of: []) + #expect(type(of: result) == Text.self) + } + + @Test func balancedConcatenationOfSingleElementReturnsThatElement() { + let only = Text("only") + let result = Text.balancedConcatenation(of: [only]) + #expect(type(of: result) == Text.self) + } + } +}