Modernize Glance baseline - #10
Conversation
WalkthroughThis PR shifts the project to Apple Silicon-only builds targeting macOS 15 and Swift 6, updating CI to assert/verify arm64-only artifacts, converting bundled KaTeX fonts to woff2-only, and adding MainActor/nonisolated/Sendable annotations across app and plugin code for Swift 6 concurrency compliance. ChangesApple Silicon-only migration
KaTeX woff2-only font packaging
Swift concurrency adoption
Estimated code review effort: 4 (Complex) | ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Modernizes the Glance fork’s baseline to macOS 15+ and Apple Silicon-only, updates Swift concurrency annotations for Swift 6, refreshes dependency pins, and trims obsolete KaTeX/Jupyter font fallbacks while keeping existing preview support.
Changes:
- Raise baseline assumptions (macOS 15+, arm64-only) across docs, build scripts, Xcode project settings, and release workflow verification.
- Address Swift 6 actor-isolation constraints via
@MainActor/nonisolated/@Sendableadjustments in app/plugin code and tests. - Refresh dependency locks/pins (SwiftPM + Go modules) and remove non-WOFF2 KaTeX font fallbacks (plus add a regression test).
Reviewed changes
Copilot reviewed 21 out of 63 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates stated macOS/architecture support and rationale. |
| QLPlugin/Views/PreviewVCs/WebPreviewVC.swift | Swift 6 isolation tweaks; adjusts resource bundle access and teardown logic. |
| QLPlugin/Views/PreviewVC.swift | Marks preview creation API as @MainActor to align with UI-thread expectations. |
| QLPlugin/Resources/jupyter/jupyter-katex.min.css | Removes fallback .woff/.ttf font references, leaving .woff2 only. |
| QLPlugin/MainVC.swift | Updates Quick Look entrypoint signature for Swift 6 (nonisolated, @Sendable). |
| QLPlugin/Extensions/String.swift | Makes regex cache nonisolated(unsafe) for Swift 6 concurrency checking. |
| mise.toml | Bumps Go toolchain pin and simplifies HTMLConverter clean patterns. |
| HTMLConverter/go.mod | Updates Go dependency versions. |
| HTMLConverter/go.sum | Updates Go dependency checksums for the new pins. |
| GlanceTests/PreviewSmokeTests.swift | Adds KaTeX font reference regression test; adjusts isolation and webview wait helpers. |
| Glance/Utils/Menu.swift | Adds @MainActor to menu item subclasses for actor-isolation correctness. |
| Glance/Shared/Utils/AppSettings.swift | Marks shared UserDefaults accessors nonisolated(unsafe) for Swift 6 checking. |
| Glance/SettingsWC.swift | Uses NSApp.activate() for activation behavior under the new baseline. |
| Glance/Extensions/URL.swift | Removes redundant import cleanup. |
| Glance/Extensions/NSMenuItem.swift | Adds @MainActor isolation to menu item extension/associated keys. |
| Glance/AppDelegate.swift | Marks app delegate @MainActor, updates activation calls, and uses Task { @MainActor … } deferral. |
| Glance.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved | Adds SwiftPM lockfile to repo for deterministic resolution. |
| Glance.xcodeproj/project.pbxproj | Sets Swift 6, arm64-only ARCHS, macOS 15 deployment target, updates build phases/resources and package requirements. |
| .woodpecker/test.yaml | Removes Woodpecker test pipeline config. |
| .woodpecker/lint.yaml | Removes Woodpecker lint pipeline config. |
| .swiftformat | Updates Swift version to 6.0 and excludes build/. |
| .gitignore | Un-ignores the committed SwiftPM lockfile path within the workspace. |
| .github/workflows/release.yml | Enforces Apple Silicon runner and verifies produced artifacts are arm64-only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| deinit { | ||
| webView?.navigationDelegate = nil | ||
| webView?.stopLoading() | ||
| MainActor.assumeIsolated { | ||
| webView?.navigationDelegate = nil | ||
| webView?.stopLoading() | ||
| } |
| @MainActor | ||
| final class PreviewSmokeTests: XCTestCase { | ||
| private var temporaryDirectory: URL! | ||
| nonisolated(unsafe) private var temporaryDirectory: URL! | ||
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
QLPlugin/MainVC.swift (1)
45-104: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBridge the
DispatchQueue.main.asyncbody intoMainActor
preparePreviewOfFileis nonisolated, but this closure still touchesself.previewFile(file:)andself.maxFileSize. Under Swift 6, wrap the body inMainActor.assumeIsolated { ... }or switch toTask {@mainactorin ... }.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@QLPlugin/MainVC.swift` around lines 45 - 104, The body of preparePreviewOfFile still accesses MainActor-isolated state from a nonisolated context inside DispatchQueue.main.async, which will trip Swift 6 isolation checks. Move the entire preview logic into a MainActor context by wrapping the closure body in MainActor.assumeIsolated { ... } or replacing the dispatch with Task { `@MainActor` in ... }, and keep the existing uses of self.maxFileSize and self.previewFile(file:) inside that isolated block.
🧹 Nitpick comments (1)
QLPlugin/Views/PreviewVC.swift (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
@MainActorto its own line to satisfy SwiftLint.SwiftLint's
attributesrule requires attributes on their own line for function declarations.♻️ Proposed style fix
protocol Preview { init() - `@MainActor` func createPreviewVC(file: File) throws -> PreviewVC + `@MainActor` + func createPreviewVC(file: File) throws -> PreviewVC }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@QLPlugin/Views/PreviewVC.swift` at line 9, Move the `@MainActor` attribute used on createPreviewVC(file:) to its own line so the function declaration conforms to SwiftLint’s attributes rule. Update the PreviewVC-related declaration in PreviewVC.swift so the attribute is separated from the func signature, keeping the same annotation and behavior while matching the required formatting.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@QLPlugin/Extensions/String.swift`:
- Line 5: The static regexCache declaration in String.swift needs its Swift
modifiers reordered to satisfy modifier_order. Update the declaration so
nonisolated(unsafe) appears before private on the regexCache property, keeping
the same NSCache<NSString, NSRegularExpression> definition.
In `@QLPlugin/Views/PreviewVCs/WebPreviewVC.swift`:
- Around line 63-68: The WebKit teardown in WebPreviewVC’s deinit is using
MainActor.assumeIsolated, which can trap if the final release happens off the
main thread. Move the cleanup for webView (navigationDelegate reset and
stopLoading) out of deinit into an explicit main-actor invalidation method, or
make the deinit isolated if the deployment target allows it, so teardown is
guaranteed to run on the main actor.
---
Outside diff comments:
In `@QLPlugin/MainVC.swift`:
- Around line 45-104: The body of preparePreviewOfFile still accesses
MainActor-isolated state from a nonisolated context inside
DispatchQueue.main.async, which will trip Swift 6 isolation checks. Move the
entire preview logic into a MainActor context by wrapping the closure body in
MainActor.assumeIsolated { ... } or replacing the dispatch with Task {
`@MainActor` in ... }, and keep the existing uses of self.maxFileSize and
self.previewFile(file:) inside that isolated block.
---
Nitpick comments:
In `@QLPlugin/Views/PreviewVC.swift`:
- Line 9: Move the `@MainActor` attribute used on createPreviewVC(file:) to its
own line so the function declaration conforms to SwiftLint’s attributes rule.
Update the PreviewVC-related declaration in PreviewVC.swift so the attribute is
separated from the func signature, keeping the same annotation and behavior
while matching the required formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3c30458-97ac-400d-9f16-3690f01e8bec
⛔ Files ignored due to path filters (41)
HTMLConverter/go.sumis excluded by!**/*.sumQLPlugin/Resources/jupyter/fonts/KaTeX_AMS-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_AMS-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Caligraphic-Bold.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Caligraphic-Bold.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Caligraphic-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Caligraphic-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Fraktur-Bold.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Fraktur-Bold.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Fraktur-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Fraktur-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Main-Bold.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Main-Bold.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Main-BoldItalic.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Main-BoldItalic.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Main-Italic.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Main-Italic.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Main-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Main-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Math-BoldItalic.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Math-BoldItalic.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Math-Italic.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Math-Italic.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_SansSerif-Bold.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_SansSerif-Bold.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_SansSerif-Italic.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_SansSerif-Italic.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_SansSerif-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_SansSerif-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Script-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Script-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Size1-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Size1-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Size2-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Size2-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Size3-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Size3-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Size4-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Size4-Regular.woffis excluded by!**/*.woffQLPlugin/Resources/jupyter/fonts/KaTeX_Typewriter-Regular.ttfis excluded by!**/*.ttfQLPlugin/Resources/jupyter/fonts/KaTeX_Typewriter-Regular.woffis excluded by!**/*.woff
📒 Files selected for processing (22)
.github/workflows/release.yml.gitignore.swiftformat.woodpecker/lint.yaml.woodpecker/test.yamlGlance.xcodeproj/project.pbxprojGlance.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedGlance/AppDelegate.swiftGlance/Extensions/NSMenuItem.swiftGlance/Extensions/URL.swiftGlance/SettingsWC.swiftGlance/Shared/Utils/AppSettings.swiftGlance/Utils/Menu.swiftGlanceTests/PreviewSmokeTests.swiftHTMLConverter/go.modQLPlugin/Extensions/String.swiftQLPlugin/MainVC.swiftQLPlugin/Resources/jupyter/jupyter-katex.min.cssQLPlugin/Views/PreviewVC.swiftQLPlugin/Views/PreviewVCs/WebPreviewVC.swiftREADME.mdmise.toml
💤 Files with no reviewable changes (3)
- .woodpecker/lint.yaml
- .woodpecker/test.yaml
- Glance/Extensions/URL.swift
| extension String { | ||
| /// Cache for compiled regular expressions to avoid recompilation on every call | ||
| private static let regexCache = NSCache<NSString, NSRegularExpression>() | ||
| private nonisolated(unsafe) static let regexCache = NSCache<NSString, NSRegularExpression>() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reorder modifiers to satisfy modifier_order.
SwiftLint flags that nonisolated should precede private.
🧹 Proposed fix
- private nonisolated(unsafe) static let regexCache = NSCache<NSString, NSRegularExpression>()
+ nonisolated(unsafe) private static let regexCache = NSCache<NSString, NSRegularExpression>()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private nonisolated(unsafe) static let regexCache = NSCache<NSString, NSRegularExpression>() | |
| nonisolated(unsafe) private static let regexCache = NSCache<NSString, NSRegularExpression>() |
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 5-5: nonisolated modifier should come before private
(modifier_order)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@QLPlugin/Extensions/String.swift` at line 5, The static regexCache
declaration in String.swift needs its Swift modifiers reordered to satisfy
modifier_order. Update the declaration so nonisolated(unsafe) appears before
private on the regexCache property, keeping the same NSCache<NSString,
NSRegularExpression> definition.
Source: Linters/SAST tools
| deinit { | ||
| webView?.navigationDelegate = nil | ||
| webView?.stopLoading() | ||
| MainActor.assumeIsolated { | ||
| webView?.navigationDelegate = nil | ||
| webView?.stopLoading() | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File map ==\n'
git ls-files 'QLPlugin/Views/PreviewVCs/*' 'QLPlugin/**/WebPreviewVC.swift' 'QLPlugin/**/PreviewVC*.swift' 'QLPlugin/**/QuickLook*' | sed 's#^`#-` #'
printf '\n== WebPreviewVC outline ==\n'
ast-grep outline QLPlugin/Views/PreviewVCs/WebPreviewVC.swift --view expanded || true
printf '\n== WebPreviewVC source (relevant slice) ==\n'
cat -n QLPlugin/Views/PreviewVCs/WebPreviewVC.swift | sed -n '1,220p'
printf '\n== Search for MainActor / deinit / WebPreviewVC usage ==\n'
rg -n "`@MainActor`|deinit|WebPreviewVC|NSViewController" QLPlugin -g '!**/DerivedData/**' || trueRepository: ranokay/glance
Length of output: 9639
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== PreviewVC protocol ==\n'
cat -n QLPlugin/Views/PreviewVC.swift
printf '\n== MainVC source (relevant slice) ==\n'
ast-grep outline QLPlugin/MainVC.swift --view expanded || true
cat -n QLPlugin/MainVC.swift | sed -n '1,220p'
printf '\n== Search for `@MainActor` on preview controller creation and other deinit patterns ==\n'
rg -n "`@MainActor`|deinit \{|DispatchQueue\.main|assumeIsolated" QLPlugin/Views QLPlugin/MainVC.swift || trueRepository: ranokay/glance
Length of output: 6209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Preview implementations ==\n'
rg -n "func createPreviewVC|class .*Preview|struct .*Preview|`@MainActor` func createPreviewVC" QLPlugin/Views/Previews QLPlugin/Views -g '*.swift' || true
printf '\n== Relevant createPreviewVC bodies ==\n'
for f in QLPlugin/Views/Previews/*.swift; do
echo "--- $f ---"
ast-grep outline "$f" --view expanded || true
doneRepository: ranokay/glance
Length of output: 9835
🌐 Web query:
Swift MainActor.assumeIsolated deinit nonisolated final release off main thread documentation
💡 Result:
In Swift, the deinitializer (deinit) of an actor or a global-actor isolated class (such as one annotated with @MainActor) is not guaranteed to execute on that actor's executor [1][2][3]. This is because the final release of an object can occur on any thread, depending on where the last strong reference is held [1][4]. If you use MainActor.assumeIsolated within a deinit, you are asserting that the current thread is already the MainActor [5]. If the deinit is triggered on a background thread, this assertion will fail and result in a runtime crash [1][5]. Therefore, it is strongly recommended not to use MainActor.assumeIsolated in deinit as a way to perform cleanup [1]. Key concepts and best practices regarding this behavior include: Isolated Deinitializers: Swift 5.10+ and later versions support isolated deinitializers (SE-0371), which allow the deinit body to be scheduled on the required actor's executor if necessary [4][6]. This is the modern, safe way to handle cleanup for isolated types. By default, unless specified, deinitializers are non-isolated [4]. Non-isolated deinit and cleanup: Because deinit is non-isolated by default, it cannot safely access isolated state or properties of the actor [4][3]. If you need to perform cleanup that requires MainActor isolation: 1. Prefer Isolated Deinitializers: If your environment supports it, ensure your deinit is treated as isolated, which handles the executor hopping automatically [4][6]. 2. Avoid using DispatchQueue.main.async: Attempting to "fix" a non-isolated deinit by dispatching to the main queue (e.g., using DispatchQueue.main.async) is unsafe because the object is already being deallocated [1]. The object might be destroyed before the block executes, leading to crashes or access to invalid memory [1]. 3. Manual Cleanup: If you cannot use isolated deinitializers, the recommended pattern is to expose an explicit cleanup method (e.g., stop, invalidate) that is called before the object is expected to be deallocated [7]. If you have code that triggers a compiler error because you are accessing isolated properties in a non-isolated deinit, it is a signal that your cleanup logic needs to be moved to an explicitly called method or that the type should be refactored to handle its own resource management [7][3]. Use of nonisolated(unsafe) properties or other workarounds to suppress these errors is generally discouraged as it risks data races [2][7].
Citations:
- 1: https://forums.swift.org/t/is-mainactor-assumeisolated-truly-necessary-in-deinit-for-a-mainactor-annotated-class/72921
- 2: https://forums.swift.org/t/unexpected-deinit-not-on-the-mainactor/81599
- 3: https://stackoverflow.com/questions/78284030/swift-5-10-cannot-access-property-with-a-non-sendable-type-from-non-iso
- 4: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0371-isolated-synchronous-deinit.md
- 5: https://developer.apple.com/documentation/swift/mainactor/assumeisolated(_:file:line:)
- 6: https://forums.swift.org/t/second-review-se-0371-isolated-synchronous-deinit/73406
- 7: https://stackoverflow.com/questions/78774528/how-to-deallocate-resource-in-main-actor-isolated-object-in-deinit
Avoid MainActor.assumeIsolated in deinit deinit is nonisolated, so this cleanup can still trap if the last release happens off-main. DispatchQueue.main.async is not a safe fallback during teardown; move this WebKit cleanup into an explicit main-actor invalidation path, or use an isolated deinit if the deployment target supports it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@QLPlugin/Views/PreviewVCs/WebPreviewVC.swift` around lines 63 - 68, The
WebKit teardown in WebPreviewVC’s deinit is using MainActor.assumeIsolated,
which can trap if the final release happens off the main thread. Move the
cleanup for webView (navigationDelegate reset and stopLoading) out of deinit
into an explicit main-actor invalidation method, or make the deinit isolated if
the deployment target allows it, so teardown is guaranteed to run on the main
actor.
Summary
Validation
Notes
xcrun simctl list devices;mise run test:xcodeno longer emits that mismatch.