Skip to content

Fix Swift 6 archive error in TextualNamespace init - #38

Closed
bluepeter wants to merge 12 commits into
gonzalezreal:mainfrom
bluepeter:main
Closed

Fix Swift 6 archive error in TextualNamespace init#38
bluepeter wants to merge 12 commits into
gonzalezreal:mainfrom
bluepeter:main

Conversation

@bluepeter

Copy link
Copy Markdown

Summary

  • Change TextualNamespace.base from let to var in Sources/Textual/TextualNamespace.swift.
  • This fixes Swift 6 + library-evolution archive builds that fail on direct assignment to a let property inside an @inlinable initializer.
  • No behavior change intended; this is a compile-compatibility fix.

Test plan

  • Build Textual-dependent app in Debug (macOS)
  • Build app for iOS Release
  • Archive app for iOS (generic/platform=iOS) succeeds

The NSTextInteractionView overlay (placed via NSViewRepresentable) can
retain stale frames after animated container resizes — for example, when
a NavigationSplitView sidebar collapses. The stale frame causes the
overlay to intercept mouse events outside its visible bounds, blocking
buttons and showing an I-beam cursor over non-text areas.

Adding .geometryGroup() between .environment(model) and
.overlayPreferenceValue ensures geometry is resolved before being
distributed to the NSView overlay, so the AppKit frame stays in sync
with the SwiftUI layout during and after animation.
AldenClark added a commit to AldenClark/textual that referenced this pull request Apr 21, 2026
# Conflicts:
#	Sources/Textual/TextualNamespace.swift
Context
-------
- Consumers (e.g. Cumbersome) crash with `EXC_BAD_ACCESS` /
  "Thread stack size exceeded due to excessive recursion" when an
  AttributedString contains many runs — most commonly a large
  fenced code block whose Prism-tokenized contents reach thousands
  of styled runs, but also paragraphs with many inline links and
  large tables.
- Crash signature is a ~2000-frame cycle of
  `LocalizedStringKey.scan` / `Text.resolve` /
  `LocalizedTextStorage.resolve`. Distinct from a 2024-era
  `ConcatenatedTextStorage` recursion seen elsewhere — different
  storage type, same class of bug.

Root cause
----------
- `TextBuilder.init` constructed one `Text` per AttributedString
  run and reduced them with `Text("\(prev)\(curr)")`. That
  initializer takes a `LocalizedStringKey` whose substitution
  arguments are the inner Texts, so the resulting tree has
  `LocalizedTextStorage` depth = run count.
- SwiftUI resolves that tree recursively at layout time, walking
  arguments via `LocalizedStringKey.scan` /
  `resolveArguments`. ~1000 levels exhausts the 1 MB main-thread
  stack on iOS.
- Building per-run was only required to attach `customAttribute`
  markers (`AttachmentAttribute`, `LinkAttribute`) that cannot
  live inside an AttributedString. Every other run carries
  standard AttributedString attributes (foregroundColor, font,
  presentationIntent, syntax-highlight tokens, link URL, etc.)
  which `Text(_ attributedString:)` preserves natively in flat
  storage.

Sources/Textual/Internal/TextFragment/TextBuilder.swift
-------------------------------------------------------
- Replace the per-run `.map` + `Text("\(prev)\(curr)")` reduction
  with a coalescing walk: plain runs (no attachment, no link)
  accumulate into a single `AttributedString`; attachment / link
  runs flush the buffer as one `Text(_ attributedString:)`, emit
  their own standalone Text with the required `customAttribute`,
  and the loop resumes buffering. Resulting Text-node count is
  bounded by `attachments + links + 1` rather than run count, so
  a 5000-token code block becomes one Text instead of 5000.
- Replace the final left-fold concat with a balanced (pairwise)
  merge via the new `Text.balancedConcatenation(of:)` helper.
  `+` produces `ConcatenatedTextStorage` rather than
  `LocalizedTextStorage` — already a major win — but a left-fold
  builds an O(N)-deep tree which would re-introduce the same
  crash class for content with thousands of attachment / link
  runs (e.g. model-generated link-list pastes). Pairwise merge
  keeps the tree O(log N) deep at the same O(N) construction
  cost.
- `runEnvironment` (font override) is now computed lazily inside
  the non-plain branch only. The 99% case (every Prism token in a
  code block, every styled span in prose) skips that struct copy
  per run.
- `balancedConcatenation(of:)` is `internal` rather than
  `fileprivate` so unit tests can exercise its edge cases without
  going through full `TextBuilder` construction.
- Added a "Recursion-safe Text construction" MARK comment
  explaining the contract so a future edit doesn't reintroduce
  `Text("\(prev)\(curr)")` in this file. The comment also notes
  why the final concat is balanced rather than left-folded.

Tests/TextualTests/Internal/TextFragment/TextBuilderTests.swift
---------------------------------------------------------------
- New test file covering `TextFragment<AttributedString>.TextBuilder`:
  - `empty`, `plainSingleRun`: smoke tests.
  - `largeRunCountConstructsCleanly`: synthesizes a 5000-run
    AttributedString with alternating foreground colors. Would
    `EXC_BAD_ACCESS` under the previous reduction even at
    construction time, since each `reduce` step builds a
    `LocalizedTextStorage` whose argument list points at the
    previous step. This is the regression guard for the primary
    fix.
  - `plainRunsAreCoalescedAcrossSizeChanges`: cycles
    `sizeChanged` on a 100-run input to verify the cache + group
    logic compose without rebuild crashes.
  - `runsWithLinks`: link-bearing input rendered through
    plain → link → plain → link → plain, verifying flush /
    standalone-emit logic on link boundaries.
  - `manyLinkRunsConstructCleanly`: 2000 link runs each forced as
    standalone Text nodes. Targets the balanced-concat path
    specifically — a left-fold across 2000+ pieces would
    re-introduce the 2024-class `ConcatenatedTextStorage`
    recursion in a different guise.
  - `balancedConcatenationOfEmptyArrayIsEmpty`,
    `balancedConcatenationOfSingleElementReturnsThatElement`:
    direct unit tests for the helper's degenerate cases (empty
    array returns `Text(verbatim: "")`; single-element passthrough
    avoids unnecessary concatenation).

Compatibility
-------------
- AttachmentOverlay / TextLinkInteraction / TextSelectionBackground
  unchanged: they walk `Text.Layout` looking for the same
  `customAttribute` markers, which are still applied to standalone
  attachment / link Texts. Pasteboard `presentationIntent` is
  preserved inside AttributedString runs (it's an AttributedString
  attribute, not a Text customAttribute) and round-trips through
  `Text(_ attributedString:)`.
- Existing snapshot tests (`StructuredText/__Snapshots__/...`)
  exercise this rendering path end-to-end. They should remain
  green; if any drift appears it would be at the snapshot-byte
  level for layouts that happened to depend on per-run Text-node
  identity (none expected).
bluepeter added 9 commits June 1, 2026 22:23
Context
-------
Foundation's `AttributedString(markdown:)` parser emits no run for a
table cell that has no text. A row such as `| | Col 1 | Col 2 | Col 3 |`
therefore yields three cell runs (column ordinals 1, 2, 3) while its
body rows yield four (ordinals 0, 1, 2, 3). The table renderer iterated
those runs positionally, so the missing leading cell collapsed every
header label one column to the left and the header no longer lined up
with the data beneath it.

Table
-----
- Add `cellRanges(_:declaredColumns:)` which builds a dense,
  column-indexed array of cell ranges using the ordinal carried by each
  run's `PresentationIntent.Kind.tableCell(columnIndex:)`. Slots with no
  run are left `nil` and render blank, keeping columns aligned across
  rows even when a cell is empty.
- Width is `max(declaredColumns, highestOrdinal + 1)` so malformed rows
  with more cells than the table declared are never truncated.
- If any run lacks an ordinal (defensive against unexpected input) fall
  back to the previous positional layout for that row, so well-formed
  tables behave exactly as before.
- `GridRow` now iterates the column-indexed ranges instead of the raw
  run indices.

TableCell
---------
- `content` becomes optional `AttributedSubstring?` so a missing cell
  renders an empty cell rather than requiring a synthesized substring.
  `label` falls back to an empty `AttributedString` and
  `indentationLevel` to 0 when content is absent.
Context
-------
- Cumbersome needs raw fenced-block text to run HTML and JavaScript
  snippets from chat in a WebView without re-parsing markdown in the app.

CodeBlockProxy
--------------
- Add `public var source: String` returning `String(content.characters)`
  so custom `CodeBlockStyle` implementations can read language + source
  from `CodeBlockStyleConfiguration` alongside `copyToPasteboard()`.
Context
-------
- Host apps need to substitute a custom CodeBlockStyle while keeping the
  rest of the GitHub structured-text bundle (lists, tables, quotes, etc.).

GitHubStyle
-----------
- Generalize to GitHubStyle<CodeBlock: CodeBlockStyle> with
  init(codeBlockStyle:).
- Preserve the default API: GitHubStyle() and .gitHub still use
  GitHubCodeBlockStyle.
Context
-------
StructuredText text-selection overlays sit above custom code-block chrome.
Buttons rendered inside CodeBlockStyle never received taps unless their
frames were registered as overflow exclusion rects (previously internal
to Overflow scroll regions only).

Textual
-------
- Add TextualNamespace.interactiveExclusionRegion() so host apps can mark
  interactive regions inside StructuredText styles without forking
  OverflowFrameKey.
@gonzalezreal

Copy link
Copy Markdown
Owner

Thanks for looking into this. I ended up merging #59, which fixes the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants