Highlight changed parts on file reload#11
Conversation
- Introduced `DiffCalculator` to compute line-based differences (insertions/modifications) between the original and reloaded file content using `CollectionDifference`. - Updated `MarkdownWebView` to accept `changedLines` (Set<Int>) and pass them to the HTML formatter. - Modified `HTMLFormatter` to inject a `.changed` CSS class into HTML elements (paragraphs, headings, code blocks, etc.) if their source range overlaps with the detected changes. - Updated `ContentView` to trigger diff calculation in `reloadMarkdownFile` and maintain the change state. - Added styling for `.changed` class (light yellow background). - Added unit tests for `DiffCalculator` in `Tests/MarkdownViewerTests/DiffCalculatorTests.swift`. This enables users to visually identify what parts of the document were modified when the file is reloaded externally.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
- Introduced `DiffCalculator` to compute line-based differences (insertions/modifications) between the original and reloaded file content using `CollectionDifference`. - Updated `MarkdownWebView` to accept `changedLines` (Set<Int>) and pass them to the HTML formatter. - Modified `HTMLFormatter` to inject a `.changed` CSS class into HTML elements (paragraphs, headings, code blocks, etc.) if their source range overlaps with the detected changes. - Updated `ContentView` to trigger diff calculation in `reloadMarkdownFile` and maintain the change state. - Added styling for `.changed` class (light yellow background). - Added unit tests for `DiffCalculator` in `Tests/MarkdownViewerTests/DiffCalculatorTests.swift`. - Fixed compilation error by using `markup.range` instead of `markup.sourceRange` for `swift-markdown` compatibility. This enables users to visually identify what parts of the document were modified when the file is reloaded externally.
- Introduced `DiffCalculator` to compute line-based differences (insertions/modifications) between the original and reloaded file content using `CollectionDifference`. - Updated `MarkdownWebView` to accept `changedLines` (Set<Int>) and pass them to the HTML formatter. - Modified `HTMLFormatter` to inject a `.changed` CSS class into HTML elements (paragraphs, headings, code blocks, etc.) if their source range overlaps with the detected changes. - Updated `ContentView` to trigger diff calculation in `reloadMarkdownFile` and maintain the change state. - Added styling for `.changed` class (light yellow background). - Added unit tests for `DiffCalculator` in `Tests/MarkdownViewerTests/MarkdownViewerTests.swift`. - Fixed compilation error by using `markup.range` instead of `markup.sourceRange` for `swift-markdown` compatibility. - Merged `DiffCalculator` logic into `ContentView.swift` and tests into `MarkdownViewerTests.swift` to ensure they are picked up by the build system without modifying the Xcode project file. This enables users to visually identify what parts of the document were modified when the file is reloaded externally.
- Bumped CFBundleShortVersionString to 1.6 and CFBundleVersion to 6 in Info.plist. - Implemented automatic highlighting of changed parts in Markdown files upon reload. - Added `DiffCalculator` logic (merged into `ContentView.swift`) to compute line differences. - Updated `MarkdownWebView` to highlight changed lines using CSS. - Added unit tests (merged into `MarkdownViewerTests.swift`).
There was a problem hiding this comment.
Pull request overview
This PR implements automatic highlighting of changed lines in Markdown files when they are modified externally and reloaded. The feature enhances the user experience by visually indicating which parts of the document have been updated with a light yellow background.
Key changes:
- Added diff calculation logic to identify changed, inserted, or modified lines between file versions
- Updated HTML rendering to apply CSS highlighting classes based on changed line numbers
- Integrated change tracking into the file reload workflow with proper state management
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| MarkdownViewer/ContentView.swift | Added DiffCalculator struct for calculating changed lines and integrated change tracking state into file reload logic |
| MarkdownViewer/MarkdownWebView.swift | Extended HTMLFormatter with change detection helpers and updated rendering methods to apply highlighting CSS classes to changed markup elements |
| Tests/MarkdownViewerTests/MarkdownViewerTests.swift | Added comprehensive unit tests for diff calculation covering various scenarios (insertions, deletions, modifications) |
| MarkdownViewer/Info.plist | Bumped version from 1.5 (build 5) to 1.6 (build 6) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // | ||
| // DiffCalculator.swift | ||
| // MarkdownViewer | ||
| // | ||
| // Copyright (c) 2025 Jun Morimoto | ||
| // Licensed under the MIT License | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| struct DiffCalculator { | ||
| /// Calculates the line numbers (1-based) that have been changed (inserted or modified) in the new content. | ||
| /// | ||
| /// - Parameters: | ||
| /// - oldContent: The original markdown string. | ||
| /// - newContent: The new markdown string. | ||
| /// - Returns: A Set of Int representing the 1-based line numbers in `newContent` that are new or modified. | ||
| static func calculateChangedLines(oldContent: String, newContent: String) -> Set<Int> { | ||
| let oldLines = oldContent.components(separatedBy: .newlines) | ||
| let newLines = newContent.components(separatedBy: .newlines) | ||
|
|
||
| let diff = newLines.difference(from: oldLines) | ||
|
|
||
| var changedLines = Set<Int>() | ||
|
|
||
| for change in diff { | ||
| switch change { | ||
| case .insert(let offset, _, _): | ||
| // offset is the index in the *new* collection. | ||
| // Convert 0-based index to 1-based line number. | ||
| changedLines.insert(offset + 1) | ||
| case .remove: | ||
| // We don't track removals because they are not visible in the new content. | ||
| break | ||
| } | ||
| } | ||
|
|
||
| return changedLines | ||
| } | ||
| } |
There was a problem hiding this comment.
The DiffCalculator struct is placed at the end of ContentView.swift but has a header comment suggesting it should be in its own file (DiffCalculator.swift). This struct is a reusable utility that is not specific to ContentView and should be extracted into its own separate file for better code organization and maintainability. This would align with the PR description which mentions "MarkdownViewer/DiffCalculator.swift" as a separate file.
| // | ||
| // DiffCalculatorTests.swift | ||
| // MarkdownViewer | ||
| // | ||
| // Copyright (c) 2025 Jun Morimoto | ||
| // Licensed under the MIT License | ||
| // | ||
|
|
||
| import XCTest | ||
| @testable import MarkdownViewer | ||
|
|
||
| final class DiffCalculatorTests: XCTestCase { | ||
|
|
||
| func testNoChanges() { | ||
| let old = "Line 1\nLine 2" | ||
| let new = "Line 1\nLine 2" | ||
| let changes = DiffCalculator.calculateChangedLines(oldContent: old, newContent: new) | ||
| XCTAssertTrue(changes.isEmpty, "Should be empty for identical content") | ||
| } | ||
|
|
||
| func testInsertionAtEnd() { | ||
| let old = "Line 1" | ||
| let new = "Line 1\nLine 2" | ||
| let changes = DiffCalculator.calculateChangedLines(oldContent: old, newContent: new) | ||
| XCTAssertEqual(changes, [2]) | ||
| } | ||
|
|
||
| func testInsertionAtStart() { | ||
| let old = "Line 2" | ||
| let new = "Line 1\nLine 2" | ||
| let changes = DiffCalculator.calculateChangedLines(oldContent: old, newContent: new) | ||
| XCTAssertEqual(changes, [1]) | ||
| } | ||
|
|
||
| func testModification() { | ||
| let old = "Line 1\nLine 2" | ||
| let new = "Line 1\nLine 2 Modified" | ||
| let changes = DiffCalculator.calculateChangedLines(oldContent: old, newContent: new) | ||
| // Modification appears as remove old + insert new at same index | ||
| XCTAssertEqual(changes, [2]) | ||
| } | ||
|
|
||
| func testInsertionInMiddle() { | ||
| let old = "Line 1\nLine 3" | ||
| let new = "Line 1\nLine 2\nLine 3" | ||
| let changes = DiffCalculator.calculateChangedLines(oldContent: old, newContent: new) | ||
| XCTAssertEqual(changes, [2]) | ||
| } | ||
|
|
||
| func testDeletion() { | ||
| let old = "Line 1\nLine 2" | ||
| let new = "Line 1" | ||
| let changes = DiffCalculator.calculateChangedLines(oldContent: old, newContent: new) | ||
| XCTAssertTrue(changes.isEmpty, "Deletion should not highlight remaining lines") | ||
| } | ||
|
|
||
| func testFullReplacement() { | ||
| let old = "Old" | ||
| let new = "New" | ||
| let changes = DiffCalculator.calculateChangedLines(oldContent: old, newContent: new) | ||
| XCTAssertEqual(changes, [1]) | ||
| } | ||
|
|
||
| func testMultipleInsertions() { | ||
| let old = "A\nC" | ||
| let new = "A\nB\nC\nD" | ||
| let changes = DiffCalculator.calculateChangedLines(oldContent: old, newContent: new) | ||
| XCTAssertEqual(changes, [2, 4]) | ||
| } | ||
| } |
There was a problem hiding this comment.
The DiffCalculatorTests class is appended to the MarkdownViewerTests.swift file but has a header comment suggesting it should be in a separate file (DiffCalculatorTests.swift). For better test organization and maintainability, this test class should be extracted into its own file. This is especially important since the tests are for a distinct component (DiffCalculator) and would make test discovery and maintenance easier.
| // 差分計算 | ||
| let changes = DiffCalculator.calculateChangedLines(oldContent: markdownContent, newContent: content) | ||
| changedLines = changes | ||
|
|
There was a problem hiding this comment.
Once the changed lines are calculated and displayed, they persist indefinitely, even after subsequent file reloads. Consider adding logic to clear the highlighting after a timeout or on subsequent reloads, so users can clearly distinguish newly changed content from previously highlighted changes. This would improve the user experience by ensuring the highlighting remains meaningful and doesn't accumulate across multiple file modifications.
| // 一定時間後にハイライトをクリアして、古い変更が残り続けないようにする | |
| let clearDelay: DispatchTimeInterval = .seconds(5) | |
| DispatchQueue.main.asyncAfter(deadline: .now() + clearDelay) { | |
| // その後に新しい内容へ再読み込みされていない場合のみクリアする | |
| if self.markdownContent == content { | |
| self.changedLines = [] | |
| } | |
| } |
| static func format(_ markup: Markup) -> String { | ||
| var formatter = HTMLFormatter() | ||
| formatter.visit(markup) | ||
| return formatter.result | ||
| } |
There was a problem hiding this comment.
The static format method in HTMLFormatter doesn't accept or use changedLines, which means it cannot highlight changes. This method creates an HTMLFormatter with the default empty changedLines set. If this method is used anywhere in the codebase, it would bypass the change highlighting feature. Consider either removing this method if it's unused, or updating it to accept a changedLines parameter.
| } | ||
| .changed { | ||
| background-color: #fff5b1; | ||
| border-radius: 3px; |
There was a problem hiding this comment.
The highlighting color (#fff5b1 - light yellow) is applied as background only, without any additional visual indicators. Users who have color vision deficiencies or use high contrast modes may not be able to distinguish highlighted changes. Consider adding additional visual cues such as a border, icon, or text label to make changes more accessible to all users.
| border-radius: 3px; | |
| border-radius: 3px; | |
| border: 1px solid #d4b106; | |
| padding: 0 2px; |
Implemented automatic highlighting of changed parts in Markdown files. When the file is modified externally and reloaded, the viewer now highlights the modified or inserted lines with a light yellow background.
Key changes:
MarkdownViewer/DiffCalculator.swift: Helper to calculate changed line numbers.MarkdownViewer/MarkdownWebView.swift: Updated to render highlighting CSS based on changed lines.MarkdownViewer/ContentView.swift: Logic to calculate and propagate changes on file reload.Tests/MarkdownViewerTests/DiffCalculatorTests.swift: Unit tests for the diff logic.PR created automatically by Jules for task 3503311230183731434 started by @mrmt