Open
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 34 minutes and 17 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR restructures the TextDiff package from a monolithic library into platform-specific modules: TextDiffCore (core diffing), TextDiffUICommon (shared cross-platform UI code), TextDiffMacOSUI (macOS-specific UI), and TextDiffIOSUI (iOS-specific UI). It replaces AppKit-specific types with cross-platform abstractions (PlatformColor, PlatformFont, TextDiffEdgeInsets) and adds iOS 18 platform support. Tests are reorganized to align with the new module structure. Changes
Sequence DiagramsequenceDiagram
participant SwiftUI as SwiftUI View
participant TextDiffView as TextDiffView<br/>(Root API)
participant Core as TextDiffCore<br/>(Engine)
participant UICommon as TextDiffUICommon<br/>(Layout)
participant macOS as macOS Path<br/>(NSViewRep)
participant iOS as iOS Path<br/>(UIViewRep)
SwiftUI->>TextDiffView: Provide text/style
TextDiffView->>TextDiffView: Detect platform
alt macOS Path
TextDiffView->>macOS: Render NSViewRepresentable
macOS->>Core: Request diff segments
Core-->>macOS: DiffSegments
macOS->>UICommon: Layout segments with style
UICommon-->>macOS: DiffLayout (runs with colors/attrs)
macOS->>macOS: Draw attributed text + chip backgrounds
else iOS Path
TextDiffView->>iOS: Render UIViewRepresentable
iOS->>Core: Request diff segments
Core-->>iOS: DiffSegments
iOS->>UICommon: Layout segments with style
UICommon-->>iOS: DiffLayout (runs with colors/attrs)
iOS->>iOS: Draw via CoreGraphics
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
Package.swift (1)
43-72:TESTINGflag scope makes#if !TESTING assert(...)blocks effectively dead.
TESTINGis defined for every debug build, not just test runs. Sinceassertis a no-op in release, the guarded assertions inDiffSegmentIndexer(and any similar guards) never execute in any configuration. If the intent is to suppress assertions only while running tests, consider moving theTESTINGdefine to the test target via an-Dunsafe flag / test plan, or use#if DEBUG && !TESTINGwith the flag set only in a dedicated configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Package.swift` around lines 43 - 72, The TESTING build define is applied to all debug targets making any guards like `#if !TESTING` in DiffSegmentIndexer effectively dead; update the build settings so TESTING is only defined for actual test runs (e.g., remove `.define("TESTING", .when(configuration: .debug))` from the targets and instead add the flag only to the test target via an unsafe `-DTESTING` test-only flag or a dedicated test configuration), or adjust the conditionals to `#if DEBUG && !TESTING` and ensure TESTING is set only in the test configuration so assertions in DiffSegmentIndexer behave as intended.Sources/TextDiffCore/TextDiff.swift (1)
1-1: Consider deleting this placeholder file.A file containing only a comment adds noise without carrying anything the compiler or reader uses. If
TextDiffCoreneeds at least one source file to compile, keep it; otherwise drop it. If kept, a brief note pointing to the actual entry points (e.g.DiffSegmentIndexer.swift,TextDiffEngine.swift) would be more useful than the current one-liner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TextDiffCore/TextDiff.swift` at line 1, This file is a no-op placeholder; either remove TextDiff.swift to eliminate noise (if TextDiffCore builds without it) or replace the comment with a concise guide pointing to the real entry points (e.g., reference DiffSegmentIndexer.swift and TextDiffEngine.swift) so readers know where the public API lives; update the repository accordingly and ensure the module still compiles after deleting or editing TextDiff.swift.Sources/TextDiff/TextDiffView.swift (1)
363-401: Add iOS previews for the new SwiftUI path.The macOS branch has previews, but the new iOS representable path has none. Add a couple of iOS
#Previews, for example default rendering and precomputed-result rendering.As per coding guidelines, SwiftUI views should always include
#Previewwith a few state configurations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TextDiff/TextDiffView.swift` around lines 363 - 401, Add SwiftUI previews for the iOS representable by creating one or more `#Preview` blocks that instantiate TextDiffIOSRepresentable with (1) live inputs using original/updated/mode/style to show default rendering and (2) a precomputed TextDiffResult passed to the result parameter to show the precomputed-result rendering; reference TextDiffIOSRepresentable (the makeUIView/updateUIView paths) and ensure the previews compile under `#if` os(iOS) and exercise both branches (result != nil and result == nil).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/TextDiffIOSUI/UITextDiffView.swift`:
- Around line 6-57: Add three public properties to
UITextDiffView—isRevertActionsEnabled: Bool, showsInvisibleCharacters: Bool, and
onRevertAction: ((DiffSegment) -> Void)?—with sensible defaults and didSet
observers that mirror NSTextDiffView semantics: when changed, set any relevant
internal state and call the same update/invalidation flow used elsewhere (e.g.
set contentSource = .text and call updateSegmentsIfNeeded() or call
invalidateCachedLayout()/mark pendingStyleInvalidation as appropriate). Ensure
the onRevertAction is stored and invoked by whatever revert-action UI/path the
view exposes, and reference existing helpers like updateSegmentsIfNeeded(),
invalidateCachedLayout(), contentSource, pendingStyleInvalidation, and segments
to locate where to hook the new behavior.
In `@Sources/TextDiffUICommon/TextDiffChangeStyleDefaults.swift`:
- Around line 9-14: The defaultRemoval TextDiffChangeStyle currently sets
strikethrough to false causing deletions to render without a strike; update the
defaultRemoval initializer (TextDiffChangeStyle defaultRemoval) to set
strikethrough: true so deleted text uses the strikethrough style by default,
leaving other properties (fillColor, strokeColor, textColorOverride) unchanged.
In `@Sources/TextDiffUICommon/TextDiffViewModel.swift`:
- Around line 3-4: Remove the duplicate import by deleting one of the repeated
"import TextDiffCore" statements in TextDiffViewModel.swift so the module is
only imported once; keep a single "import TextDiffCore" line and remove the
redundant one.
---
Nitpick comments:
In `@Package.swift`:
- Around line 43-72: The TESTING build define is applied to all debug targets
making any guards like `#if !TESTING` in DiffSegmentIndexer effectively dead;
update the build settings so TESTING is only defined for actual test runs (e.g.,
remove `.define("TESTING", .when(configuration: .debug))` from the targets and
instead add the flag only to the test target via an unsafe `-DTESTING` test-only
flag or a dedicated test configuration), or adjust the conditionals to `#if
DEBUG && !TESTING` and ensure TESTING is set only in the test configuration so
assertions in DiffSegmentIndexer behave as intended.
In `@Sources/TextDiff/TextDiffView.swift`:
- Around line 363-401: Add SwiftUI previews for the iOS representable by
creating one or more `#Preview` blocks that instantiate TextDiffIOSRepresentable
with (1) live inputs using original/updated/mode/style to show default rendering
and (2) a precomputed TextDiffResult passed to the result parameter to show the
precomputed-result rendering; reference TextDiffIOSRepresentable (the
makeUIView/updateUIView paths) and ensure the previews compile under `#if` os(iOS)
and exercise both branches (result != nil and result == nil).
In `@Sources/TextDiffCore/TextDiff.swift`:
- Line 1: This file is a no-op placeholder; either remove TextDiff.swift to
eliminate noise (if TextDiffCore builds without it) or replace the comment with
a concise guide pointing to the real entry points (e.g., reference
DiffSegmentIndexer.swift and TextDiffEngine.swift) so readers know where the
public API lives; update the repository accordingly and ensure the module still
compiles after deleting or editing TextDiff.swift.
🪄 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: 9add5936-e6ca-4644-b093-7beb1377b8f9
⛔ Files ignored due to path filters (18)
Tests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/character_mode_no_affordance.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/character_suffix_refinement.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/custom_style_spacing_strikethrough.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/hover_pair_affordance.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/hover_single_addition_affordance.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/hover_single_deletion_affordance.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/invisible_characters_debug_overlay.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/multiline_insertion_wrap.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/narrow_width_wrapping.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/punctuation_replacement.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/NSTextDiffSnapshotTests/token_basic_replacement.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/TextDiffSnapshotTests/character_suffix_refinement.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/TextDiffSnapshotTests/custom_style_spacing_strikethrough.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/TextDiffSnapshotTests/multiline_insertion_wrap.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/TextDiffSnapshotTests/precomputed_result_rendering.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/TextDiffSnapshotTests/punctuation_replacement.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/TextDiffSnapshotTests/token_basic_replacement.1.pngis excluded by!**/*.pngTests/TextDiffMacOSUITests/__Snapshots__/TextDiffSnapshotTests/whitespace_only_layout_change.1.pngis excluded by!**/*.png
📒 Files selected for processing (34)
Package.swiftSources/TextDiff/TextDiff.swiftSources/TextDiff/TextDiffChangeStyleDefaults.swiftSources/TextDiff/TextDiffView.swiftSources/TextDiffCore/DiffSegmentIndexer.swiftSources/TextDiffCore/DiffTypes.swiftSources/TextDiffCore/MyersDiff.swiftSources/TextDiffCore/TextDiff.swiftSources/TextDiffCore/TextDiffEngine.swiftSources/TextDiffCore/TextDiffResult.swiftSources/TextDiffCore/Tokenizer.swiftSources/TextDiffIOSUI/UITextDiffView.swiftSources/TextDiffMacOSUI/AppKit/DiffRevertActionResolver.swiftSources/TextDiffMacOSUI/AppKit/DiffTextViewRepresentable.swiftSources/TextDiffMacOSUI/AppKit/NSTextDiffContentSource.swiftSources/TextDiffMacOSUI/AppKit/NSTextDiffView.swiftSources/TextDiffUICommon/DiffTextLayoutMetrics.swiftSources/TextDiffUICommon/DiffTokenLayouter.swiftSources/TextDiffUICommon/TextDiffChangeStyle.swiftSources/TextDiffUICommon/TextDiffChangeStyleDefaults.swiftSources/TextDiffUICommon/TextDiffGroupStrokeStyle.swiftSources/TextDiffUICommon/TextDiffPlatformTypes.swiftSources/TextDiffUICommon/TextDiffStyle.swiftSources/TextDiffUICommon/TextDiffStyling.swiftSources/TextDiffUICommon/TextDiffViewModel.swiftTests/TextDiffCoreTests/TextDiffEngineTests.swiftTests/TextDiffMacOSUITests/DiffLayouterPerformanceTests.swiftTests/TextDiffMacOSUITests/DiffRevertActionResolverTests.swiftTests/TextDiffMacOSUITests/NSTextDiffSnapshotTests.swiftTests/TextDiffMacOSUITests/NSTextDiffViewTests.swiftTests/TextDiffMacOSUITests/SnapshotTestSupport.swiftTests/TextDiffMacOSUITests/TextDiffSnapshotTests.swiftTests/TextDiffMacOSUITests/TextDiffStyleAndLayouterTests.swiftTests/TextDiffMacOSUITests/TextDiffViewModelTests.swift
💤 Files with no reviewable changes (1)
- Sources/TextDiff/TextDiffChangeStyleDefaults.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request restructures the
TextDiffpackage to support both macOS and iOS platforms, modularizes the codebase into platform-specific and shared components, and updates the SwiftUI interface to be cross-platform. The most significant changes include the introduction of new library and target modules, refactoring of the main SwiftUI view to support both platforms, and moving core logic into a newTextDiffCoretarget.Cross-platform and modularization updates:
Package.swift) now supports both macOS and iOS, and defines new library products and targets:TextDiffCore,TextDiffUICommon,TextDiffMacOSUI, andTextDiffIOSUI, along with platform-conditional dependencies and test targets. [1] [2]TextDifftarget now re-exports the core and UI modules, and conditionally includes the macOS or iOS UI module depending on the platform. (Sources/TextDiff/TextDiff.swift)SwiftUI view refactorings:
TextDiffViewinSources/TextDiff/TextDiffView.swiftis refactored to use platform-conditional logic, delegating to either a macOS (TextDiffMacOSRepresentable) or iOS (TextDiffIOSRepresentable) implementation. This enables the same SwiftUI interface to work seamlessly across platforms. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]PlatformFontandTextDiffEdgeInsetsare now used in place of AppKit-only types, making the style configuration cross-platform. [1] [2] [3] [4]Core logic reorganization:
DiffSegmentIndexerand related types) has been moved fromSources/TextDiffto a newSources/TextDiffCoretarget, and access levels have been updated to use Swift's newpackagevisibility.Sources/TextDiffCore/TextDiff.swiftfor clarity.Cleanup and removals:
TextDiffChangeStyleDefaults.swift, further decoupling platform-specific code.These changes lay the foundation for a clean, modular, and cross-platform Swift package architecture for text diffing.
Summary by CodeRabbit
New Features
Changes