Add DiffParsing protocol for custom diff formats#9
Conversation
Make DiffFile / DiffHunk / DiffLine / LineType public with public initializers so consumers can construct the renderer's domain model directly. Introduce a public `DiffParsing` protocol; `DiffRenderer` now reads the active parser from the environment via a new `.diffParser(_:)` view modifier. The built-in unified-diff parser ships as `UnifiedDiffParser` and remains the environment default, so existing callers see no change. This lets downstream apps render formats other than standard unified diff (annotated diffs, server-side payloads, JSON patches, language- server output, ...) without modifying the library: implement `DiffParsing`, map your format to `[DiffFile]`, inject it. Tests cover parity with the legacy parser, custom-parser construction via the public initializers, the env default, and the empty-result path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR enables custom diff parsing in gitdiff by exposing ChangesCustom Diff Parser Extensibility
Sequence DiagramsequenceDiagram
participant SwiftUI as SwiftUI App
participant DiffRenderer
participant Environment as EnvironmentValues
participant Parser as DiffParsing
participant UnifiedDiffParser
participant DiffParser
SwiftUI->>DiffRenderer: diffText provided
DiffRenderer->>Environment: read \.diffParser
Environment-->>DiffRenderer: active parser (default: UnifiedDiffParser)
DiffRenderer->>Parser: parser.parse(diffText)
alt Custom Parser
Parser->>Parser: custom parse logic
else Unified Default
Parser->>UnifiedDiffParser: delegate
UnifiedDiffParser->>DiffParser: parse(diffText)
DiffParser-->>UnifiedDiffParser: [DiffFile]
end
Parser-->>DiffRenderer: [DiffFile]
DiffRenderer->>DiffRenderer: update parsedFiles state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/gitdiff/Models/DiffLine.swift (1)
34-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMove documentation comments above each case; trailing
///are not recognized as DocC documentation.With
LineTypenow public, documentation comments need to precede the declaration to be picked up by DocC and appear in generated docs and Quick Help. Trailing///comments on the same line as each case are treated as regular comments and will be invisible to both documentation generation and IDE Quick Help.📝 Proposed fix
/// Type of diff line. public enum LineType: Sendable { - case added /// Line was added (+) - case removed /// Line was removed (-) - case context /// Unchanged context line - case header /// Section header + /// Line was added (+). + case added + /// Line was removed (-). + case removed + /// Unchanged context line. + case context + /// Section header. + case header }🤖 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 `@Sources/gitdiff/Models/DiffLine.swift` around lines 34 - 39, The trailing inline comments on the LineType cases are not picked up by DocC; move each documentation comment to its own line above the corresponding enum case so DocC/Quick Help recognizes them — update the enum LineType so that /// Line was added (+) precedes case added, /// Line was removed (-) precedes case removed, /// Unchanged context line precedes case context, and /// Section header precedes case header (preserving wording).
🧹 Nitpick comments (1)
Sources/gitdiff/Models/DiffFile.swift (1)
18-49: ⚡ Quick winConsider conforming public models to
Equatable/Hashable.Now that
DiffFile,DiffHunk,DiffLine, andDiffLine.LineTypeare part of the public surface, callers (and the newDiffParsingTestsparity assertions) will likely benefit from value-equality and hashing. All stored properties are value types andEquatable-by-default, so the conformances are essentially free.Note: the synthesized
Equatableincludesid, which means twoDiffFiles with identical content but differentUUIDs would compare unequal. If you want content-based equality (useful for legacy-parser parity tests), provide an explicit==that ignoresid.♻️ Proposed conformance additions
-public struct DiffFile: Identifiable, Sendable { +public struct DiffFile: Identifiable, Sendable, Hashable {Apply the same
Hashable(which impliesEquatable) addition toDiffHunk,DiffLine, andDiffLine.LineType.🤖 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 `@Sources/gitdiff/Models/DiffFile.swift` around lines 18 - 49, Add synthesized Equatable/Hashable conformances for the public models DiffFile, DiffHunk, DiffLine, and DiffLine.LineType by making each type conform to Hashable (which also provides Equatable); ensure all stored value-type properties are included so callers and DiffParsingTests can compare/hash instances. For DiffFile decide equality semantics: the synthesized Equatable will include id (UUID) which makes files with identical content but different ids unequal — if you need content-based equality for parity tests, implement an explicit static func == in DiffFile that compares oldPath, newPath, hunks, isBinary, and isRenamed but ignores id, and also implement Hashable accordingly so hashing matches the chosen equality.
🤖 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 `@Sources/gitdiff/Views/DiffRenderer.swift`:
- Around line 70-72: The Task is currently keyed only by diffText so updating
the .diffParser at runtime won’t retrigger parsing; change the .task(id:) key to
include the parser as well (e.g., use a tuple of diffText and the parser
identity) so that when the parser instance changes the task restarts and
parsedFiles is refreshed; you can either make the parser Hashable and use id:
(diffText, parser) or use id: (diffText, ObjectIdentifier(parser as AnyObject))
and keep the body calling self.parsedFiles = try? await parser.parse(diffText).
---
Outside diff comments:
In `@Sources/gitdiff/Models/DiffLine.swift`:
- Around line 34-39: The trailing inline comments on the LineType cases are not
picked up by DocC; move each documentation comment to its own line above the
corresponding enum case so DocC/Quick Help recognizes them — update the enum
LineType so that /// Line was added (+) precedes case added, /// Line was
removed (-) precedes case removed, /// Unchanged context line precedes case
context, and /// Section header precedes case header (preserving wording).
---
Nitpick comments:
In `@Sources/gitdiff/Models/DiffFile.swift`:
- Around line 18-49: Add synthesized Equatable/Hashable conformances for the
public models DiffFile, DiffHunk, DiffLine, and DiffLine.LineType by making each
type conform to Hashable (which also provides Equatable); ensure all stored
value-type properties are included so callers and DiffParsingTests can
compare/hash instances. For DiffFile decide equality semantics: the synthesized
Equatable will include id (UUID) which makes files with identical content but
different ids unequal — if you need content-based equality for parity tests,
implement an explicit static func == in DiffFile that compares oldPath, newPath,
hunks, isBinary, and isRenamed but ignores id, and also implement Hashable
accordingly so hashing matches the chosen equality.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e3296d4-021d-4797-a6b2-a961e796daf0
📒 Files selected for processing (8)
README.mdSources/gitdiff/Core/DiffParsing.swiftSources/gitdiff/DiffEnvironment.swiftSources/gitdiff/Models/DiffFile.swiftSources/gitdiff/Models/DiffHunk.swiftSources/gitdiff/Models/DiffLine.swiftSources/gitdiff/Views/DiffRenderer.swiftTests/gitdiffTests/DiffParsingTests.swift
| .task(id: diffText) { | ||
| self.parsedFiles = try? await DiffParser.parse(diffText) | ||
| self.parsedFiles = try? await parser.parse(diffText) | ||
| } |
There was a problem hiding this comment.
Re-parse when parser changes, not only when diffText changes.
Line 70 keys the task only by diffText, so changing .diffParser(...) at runtime won’t trigger a new parse and can show stale output.
Suggested adjustment
- .task(id: diffText) {
+ .task(id: "\(diffText)|\(String(reflecting: type(of: parser)))") {
self.parsedFiles = try? await parser.parse(diffText)
}📝 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.
| .task(id: diffText) { | |
| self.parsedFiles = try? await DiffParser.parse(diffText) | |
| self.parsedFiles = try? await parser.parse(diffText) | |
| } | |
| .task(id: "\(diffText)|\(String(reflecting: type(of: parser)))") { | |
| self.parsedFiles = try? await parser.parse(diffText) | |
| } |
🤖 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 `@Sources/gitdiff/Views/DiffRenderer.swift` around lines 70 - 72, The Task is
currently keyed only by diffText so updating the .diffParser at runtime won’t
retrigger parsing; change the .task(id:) key to include the parser as well
(e.g., use a tuple of diffText and the parser identity) so that when the parser
instance changes the task restarts and parsedFiles is refreshed; you can either
make the parser Hashable and use id: (diffText, parser) or use id: (diffText,
ObjectIdentifier(parser as AnyObject)) and keep the body calling
self.parsedFiles = try? await parser.parse(diffText).
Summary
public protocol DiffParsingso consumers can plug in formats other than standard unified diff (annotated diffs, server-side payloads, JSON patches, language-server output, …) without forking the library.DiffFile,DiffHunk,DiffLine, andDiffLine.LineTypepublicwith public initialisers, so custom parsers can build the renderer's domain model directly..diffParser(_:)view modifier following the existing.diffTheme/.diffLineNumbersenv-driven pattern.UnifiedDiffParserwraps today's parser and remains the environment default — no behaviour change for existing callers.Why
Today the renderer is hard-wired to
DiffParser.parse(...)so any consumer with a non-unified format has to either convert their data to unified diff first (lossy, fragile) or fork the library. A small extension point keeps the library focused on rendering while letting downstream apps own their own parsing logic.Usage
Test plan
swift buildcleanswift test— 11/11 pass, including newDiffParsingTests(parity with legacy parser, custom-parser construction via public inits, env default, empty-result path)DiffParserTests(7) still pass unchangedDiffRenderer(diffText:).diffTheme(.dark)/.diffLineNumbers(...)calls work as before🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
.diffParser()modifier, allowing injection of custom parsing implementations.Documentation