prism branch worktree-fix-macos-build-a11y-footnote commits 1 files 1 touched lines +1 / -8

Pre-push review: macOS build fix (footnote a11y)

One-line revert that realigns FootnoteAccessibility.inlinePhrase(for:) with the spec-prescribed String(localized:) form, restoring the macOS and iOS builds broken by PR #256.

At a glance

  • Root cause: commit 974b6a6 (T-1036) introduced an unsupported overload combination — String(localized:) with interpolation cannot take a defaultValue: argument.
  • Fix: drop defaultValue:; rely on the existing a11y.footnote.inline %lld catalog entry which is translated for en, en-GB, and en-US.
  • Spec alignment: the spec's implementation.md prescribes exactly this call form; the broken commit diverged.
  • Verified: make build-macos, make build-ios, make lint, and the FootnoteAccessibilityTests suite all pass.

Verdict

Ready to push

Minimal mechanical revert. macOS build, iOS build, SwiftLint, and the existing FootnoteAccessibilityTests suite all pass. Implementation now matches specs/footnote-badge-a11y/implementation.md:141.

Commits

Three-level explanation

String(localized:) has two flavours: one accepts an interpolated key (e.g. "... \(number)") and looks up the synthesised %lld entry in the string catalog; the other accepts a fixed key plus a defaultValue: fallback. They can't be combined.

The commit that added VoiceOver support for footnote badges tried to mix them, so the project stopped compiling on macOS and iOS. This fix removes the fallback argument and uses the interpolated form alone — which is what the spec already prescribed.

Swift resolves String(localized:) to a specific overload at compile time:

  • The variant taking String.LocalizationValue supports interpolation (producing a %lld-formatted catalog key) but offers no defaultValue: parameter.
  • The variant taking StaticString + defaultValue: does support a fallback, but its key argument cannot be interpolated.

The original author wanted both — interpolation and a "footnote N" safety net — which isn't available in any single overload. Since Localizable.xcstrings already has "a11y.footnote.inline %lld" translated for en, en-GB, and en-US, the fallback was redundant; dropping it matches the spec and restores compilation.

The interpolated overload's catalog-miss fallback formats the interpolation literal with the actual value, so a missing entry would render "a11y.footnote.inline 3" — ugly, but not crashy. The author tried to harden that fallback to "footnote N" but selected an incompatible overload, producing cannot convert value of type 'String' to expected argument type 'StaticString' at FootnoteAccessibility.swift:98.

Realistic risk of a catalog miss here is near zero (the entry has extractionState: manual and three English variants present), so the safety net costs more than it buys. If a hardened fallback is ever needed in the future, the right shape is String.localizedStringWithFormat(NSLocalizedString("a11y.footnote.inline %lld", value: "footnote %lld", comment: ""), displayNumber) — but that's over-engineered for this site.

Important changes — detailed

Drop unsupported defaultValue: on interpolated String(localized:)

FootnoteAccessibility.swift

Why it matters. Restores the macOS and iOS builds, both of which fail on origin/main at this call site.

What to look at. prism/Services/FootnoteAccessibility.swift:96-98

Takeaway. String(localized:) overloads are mutually exclusive: interpolation OR defaultValue, never both. When you want a typed catalog fallback for an interpolated key, you have to drop down to NSLocalizedString + String.localizedStringWithFormat.
Rationale. The spec's implementation.md:141 explicitly prescribes the interpolation-only form, and Localizable.xcstrings already has the key translated for all three English variants — so the defaultValue: the original commit added was both incompatible with the chosen overload and redundant against the catalog.

Key decisions

Realign with existing decision log entry rather than write a new one

The original feature already has Decision 1 ("Rebuild paragraph accessibility label in Prism, not in the Textual fork") capturing the architectural choice. This fix doesn't make a new decision — it reverts a code-level deviation from the prescribed call form. No decision log update warranted.

Per-file diffs

Click to expand.

prism/Services/FootnoteAccessibility.swift Modified +1 / -8
diff --git a/prism/Services/FootnoteAccessibility.swift b/prism/Services/FootnoteAccessibility.swift
index fc5ce2c..683d459 100644
--- a/prism/Services/FootnoteAccessibility.swift
+++ b/prism/Services/FootnoteAccessibility.swift
@@ -89,14 +89,7 @@ enum FootnoteAccessibility {
     }

     /// Localised inline phrase for a footnote with the given display number.
-    ///
-    /// `defaultValue:` guards against a catalog lookup gap (missing locale,
-    /// future key rename) so VoiceOver falls back to "footnote N" instead of
-    /// announcing the raw key string.
     private static func inlinePhrase(for displayNumber: Int) -> String {
-        String(
-            localized: "a11y.footnote.inline \(displayNumber)",
-            defaultValue: "footnote \(displayNumber)"
-        )
+        String(localized: "a11y.footnote.inline \(displayNumber)")
     }
 }

Things to double-check

Catalog miss behaviour

If Localizable.xcstrings ever loses the a11y.footnote.inline %lld entry, VoiceOver will narrate "a11y.footnote.inline 3" instead of "footnote 3". The build script Tools/validate-localisation.py should catch a regression there, but worth keeping in mind if the key is ever renamed.