From a5bc81472889bd064da4491f6d687ff518be4ed9 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 06:18:09 +0300 Subject: [PATCH 1/4] feat: add expandable inline diff hunk viewer with toolbar (#689) Add a compact floating toolbar that appears above expanded diff hunks, providing navigation (prev/next), restore, and dismiss actions. - HunkToolbarView: pill-shaped AppKit overlay with SF Symbol buttons - HunkToolbarAction: enum for toolbar actions with accessibility IDs - InlineDiffProvider: hunk navigation, position info, summary, line range - GutterTextView: click-outside-hunk dismissal, onHunkDismissed callback - Toolbar repositions on scroll, hides on dismiss/data change - 37 unit tests covering navigation, dismissal, edge cases --- Pine/CodeEditorView.swift | 245 ++++++++++++- Pine/HunkToolbarAction.swift | 21 ++ Pine/HunkToolbarView.swift | 221 +++++++++++ Pine/InlineDiffProvider.swift | 50 +++ PineTests/InlineDiffHunkViewerTests.swift | 427 ++++++++++++++++++++++ 5 files changed, 953 insertions(+), 11 deletions(-) create mode 100644 Pine/HunkToolbarAction.swift create mode 100644 Pine/HunkToolbarView.swift create mode 100644 PineTests/InlineDiffHunkViewerTests.swift diff --git a/Pine/CodeEditorView.swift b/Pine/CodeEditorView.swift index 9a09bf4..da354dd 100644 --- a/Pine/CodeEditorView.swift +++ b/Pine/CodeEditorView.swift @@ -51,9 +51,17 @@ final class GutterTextView: NSTextView { /// The ID of the currently expanded hunk (shows inline diff). Nil = all collapsed. var expandedHunkID: UUID? { - didSet { needsDisplay = true } + didSet { + needsDisplay = true + if expandedHunkID == nil { + onHunkDismissed?() + } + } } + /// Callback invoked when the expanded hunk is dismissed (for toolbar cleanup). + var onHunkDismissed: (() -> Void)? + /// Subtle green tint for added lines. private static let addedLineColor = NSColor(name: nil) { appearance in if appearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua { @@ -501,25 +509,75 @@ final class GutterTextView: NSTextView { insertText("\n\(indent)", replacementRange: selectedRange()) } + // MARK: - Click outside hunk dismisses expanded hunk + + override func mouseDown(with event: NSEvent) { + if expandedHunkID != nil { + // Check if click is outside the expanded hunk range + let point = convert(event.locationInWindow, from: nil) + let clickedLine = lineNumberAtPoint(point) + if let line = clickedLine, shouldDismissHunkOnClick(atLine: line) { + dismissExpandedHunk() + } + } + super.mouseDown(with: event) + } + + /// Returns the 1-based line number at the given point in text view coordinates. + private func lineNumberAtPoint(_ point: NSPoint) -> Int? { + guard let layoutManager = layoutManager, + let textContainer = textContainer else { return nil } + let textPoint = NSPoint( + x: point.x - textContainerOrigin.x, + y: point.y - textContainerOrigin.y + ) + let glyphIndex = layoutManager.glyphIndex(for: textPoint, in: textContainer) + let charIndex = layoutManager.characterIndexForGlyph(at: glyphIndex) + let source = string as NSString + guard charIndex <= source.length else { return nil } + var line = 1 + for i in 0.. Bool { + guard let expandedID = expandedHunkID, + let hunk = diffHunksForHighlight.first(where: { $0.id == expandedID }) else { + return false + } + let range = InlineDiffProvider.expandedLineRange(for: hunk) + return !range.contains(line) + } + + /// Dismisses the currently expanded hunk, syncing both GutterTextView and LineNumberView. + func dismissExpandedHunk() { + expandedHunkID = nil + if let container = enclosingScrollView?.superview as? EditorContainerView { + for subview in container.subviews { + if let lineNumberView = subview as? LineNumberView { + lineNumberView.expandedHunkID = nil + break + } + } + } + } } // MARK: - Editor scroll view with find bar height tracking @@ -790,6 +848,11 @@ struct CodeEditorView: NSViewRepresentable { context.coordinator.lastFontSize = editorFont.pointSize context.coordinator.syncContentVersion() + // Wire up hunk dismiss callback so toolbar hides when hunk collapses from any source + textView.onHunkDismissed = { [weak coordinator] in + coordinator?.hideHunkToolbar() + } + textView.string = text if useViewportHighlighting { // Layout not yet complete — highlight first screenful asynchronously @@ -1066,6 +1129,9 @@ struct CodeEditorView: NSViewRepresentable { /// Callback for reverting a hunk via gutter button click. var onRevertHunk: ((DiffHunk) -> Void)? + /// The floating toolbar overlay shown above the expanded hunk. + private var hunkToolbar: HunkToolbarView? + /// Generation counter for cancelling stale async highlight requests. let highlightGeneration = HighlightGeneration() @@ -1610,6 +1676,17 @@ struct CodeEditorView: NSViewRepresentable { @objc func scrollViewDidScroll(_ notification: Notification) { reportStateChange() highlightOnScrollIfNeeded() + repositionHunkToolbarIfNeeded() + } + + /// Repositions the hunk toolbar after scroll so it stays aligned to the expanded hunk. + private func repositionHunkToolbarIfNeeded() { + guard let toolbar = hunkToolbar, !toolbar.isHidden, + let sv = scrollView, + let gutterView = sv.documentView as? GutterTextView, + let expandedID = gutterView.expandedHunkID, + let hunk = parent.diffHunks.first(where: { $0.id == expandedID }) else { return } + positionToolbar(toolbar, for: hunk, in: gutterView) } /// Подсвечивает видимую область при скролле (для больших файлов). @@ -1823,6 +1900,152 @@ struct CodeEditorView: NSViewRepresentable { let newID: UUID? = (gutterView.expandedHunkID == hunk.id) ? nil : hunk.id gutterView.expandedHunkID = newID lineNumberView?.expandedHunkID = newID + + if newID != nil { + showHunkToolbar(for: hunk, in: gutterView) + } else { + hideHunkToolbar() + } + } + + /// Shows the hunk toolbar overlay above the expanded hunk's first line. + func showHunkToolbar(for hunk: DiffHunk, in textView: GutterTextView) { + let toolbar: HunkToolbarView + if let existing = hunkToolbar { + toolbar = existing + } else { + toolbar = HunkToolbarView() + toolbar.onAction = { [weak self] action in + self?.handleHunkToolbarAction(action) + } + hunkToolbar = toolbar + } + + // Update summary text + let hunks = parent.diffHunks + let posInfo = InlineDiffProvider.hunkPositionInfo(for: hunk, in: hunks) + let summary = InlineDiffProvider.hunkSummary(hunk) + let position = posInfo.map { "\($0.index)/\($0.total)" } ?? "" + toolbar.summaryText = [position, summary].filter { !$0.isEmpty }.joined(separator: " ") + + // Position toolbar above the hunk's first line + positionToolbar(toolbar, for: hunk, in: textView) + + // Add to the container view (not the scroll view — so it stays fixed relative to scroll) + if toolbar.superview == nil, + let container = textView.enclosingScrollView?.superview { + container.addSubview(toolbar) + } + toolbar.isHidden = false + } + + /// Positions the toolbar at the right edge of the editor, just above the expanded hunk. + private func positionToolbar(_ toolbar: HunkToolbarView, for hunk: DiffHunk, in textView: GutterTextView) { + guard let layoutManager = textView.layoutManager, + let scrollView = textView.enclosingScrollView else { return } + + let source = textView.string as NSString + guard source.length > 0 else { return } + + // Find the character offset for the hunk's start line + let targetLine = hunk.newStart + var charOffset = 0 + var currentLine = 1 + while currentLine < targetLine && charOffset < source.length { + if source.character(at: charOffset) == ASCII.newline { + currentLine += 1 + } + charOffset += 1 + } + + let glyphIndex = layoutManager.glyphIndexForCharacter(at: min(charOffset, source.length - 1)) + let lineRect = layoutManager.lineFragmentRect(forGlyphAt: glyphIndex, effectiveRange: nil) + let visibleRect = scrollView.contentView.bounds + let originY = textView.textContainerOrigin.y + + // Y position: above the first line of the hunk, in container coordinates + let lineY = lineRect.origin.y + originY - visibleRect.origin.y + let toolbarSize = toolbar.idealSize() + let gutterWidth = textView.gutterInset + + // Right-align within the editor area (leaving space for minimap) + let containerWidth = scrollView.frame.width + let toolbarX = max(gutterWidth, containerWidth - toolbarSize.width - 8) + let toolbarY = max(0, lineY - toolbarSize.height - 2) + + toolbar.frame = NSRect( + x: toolbarX, + y: toolbarY, + width: toolbarSize.width, + height: toolbarSize.height + ) + } + + /// Hides and removes the hunk toolbar overlay. + func hideHunkToolbar() { + hunkToolbar?.isHidden = true + hunkToolbar?.removeFromSuperview() + } + + /// Handles actions from the hunk toolbar (navigation, restore, dismiss). + private func handleHunkToolbarAction(_ action: HunkToolbarAction) { + guard let sv = scrollView, + let gutterView = sv.documentView as? GutterTextView else { return } + + let hunks = parent.diffHunks + + switch action { + case .previousHunk: + guard let expandedID = gutterView.expandedHunkID, + let current = hunks.first(where: { $0.id == expandedID }), + let prev = InlineDiffProvider.previousHunk(before: current, in: hunks) else { return } + gutterView.expandedHunkID = prev.id + lineNumberView?.expandedHunkID = prev.id + showHunkToolbar(for: prev, in: gutterView) + scrollToHunk(prev, in: gutterView) + + case .nextHunk: + guard let expandedID = gutterView.expandedHunkID, + let current = hunks.first(where: { $0.id == expandedID }), + let next = InlineDiffProvider.nextHunk(after: current, in: hunks) else { return } + gutterView.expandedHunkID = next.id + lineNumberView?.expandedHunkID = next.id + showHunkToolbar(for: next, in: gutterView) + scrollToHunk(next, in: gutterView) + + case .restore: + guard let expandedID = gutterView.expandedHunkID, + let hunk = hunks.first(where: { $0.id == expandedID }) else { return } + // Dismiss toolbar before revert to avoid stale state + gutterView.expandedHunkID = nil + lineNumberView?.expandedHunkID = nil + hideHunkToolbar() + onRevertHunk?(hunk) + + case .dismiss: + gutterView.dismissExpandedHunk() + hideHunkToolbar() + } + } + + /// Scrolls the editor to make the given hunk visible. + private func scrollToHunk(_ hunk: DiffHunk, in textView: GutterTextView) { + let source = textView.string as NSString + guard source.length > 0, + let layoutManager = textView.layoutManager else { return } + + var charOffset = 0 + var currentLine = 1 + while currentLine < hunk.newStart && charOffset < source.length { + if source.character(at: charOffset) == ASCII.newline { + currentLine += 1 + } + charOffset += 1 + } + + let glyphIndex = layoutManager.glyphIndexForCharacter(at: min(charOffset, source.length - 1)) + let lineRect = layoutManager.lineFragmentRect(forGlyphAt: glyphIndex, effectiveRange: nil) + textView.scrollToVisible(lineRect) } /// Handles fold code notifications from menu/keyboard shortcuts. diff --git a/Pine/HunkToolbarAction.swift b/Pine/HunkToolbarAction.swift new file mode 100644 index 0000000..ade0603 --- /dev/null +++ b/Pine/HunkToolbarAction.swift @@ -0,0 +1,21 @@ +// +// HunkToolbarAction.swift +// Pine +// +// Actions available in the inline diff hunk toolbar (#689). +// + +import Foundation + +/// Actions available in the hunk viewer toolbar overlay. +enum HunkToolbarAction: String, Sendable { + case previousHunk + case nextHunk + case restore + case dismiss + + /// Accessibility identifier for UI testing. + var accessibilityID: String { + "hunk-toolbar-\(rawValue.replacingOccurrences(of: "Hunk", with: "").lowercased())" + } +} diff --git a/Pine/HunkToolbarView.swift b/Pine/HunkToolbarView.swift new file mode 100644 index 0000000..2fc318a --- /dev/null +++ b/Pine/HunkToolbarView.swift @@ -0,0 +1,221 @@ +// +// HunkToolbarView.swift +// Pine +// +// A compact AppKit toolbar overlay for expanded diff hunks (#689). +// Shows: ← Prev | ↑ summary (2/5) ↓ | Restore | ✕ +// Positioned above the first line of the expanded hunk, right-aligned. +// + +import AppKit + +/// A compact, pill-shaped toolbar shown above an expanded inline diff hunk. +/// Contains navigation arrows, hunk summary, restore button, and dismiss button. +final class HunkToolbarView: NSView { + + // MARK: - State + + /// Descriptive text like "2/5 +3 -1". + var summaryText: String = "" { + didSet { summaryLabel.stringValue = summaryText } + } + + /// Callback for toolbar actions. + var onAction: ((HunkToolbarAction) -> Void)? + + // MARK: - Subviews + + private let prevButton = NSButton() + private let nextButton = NSButton() + private let summaryLabel = NSTextField(labelWithString: "") + private let restoreButton = NSButton() + private let dismissButton = NSButton() + private let stackView = NSStackView() + + // MARK: - Constants + + static let toolbarHeight: CGFloat = 24 + private static let cornerRadius: CGFloat = 6 + private static let horizontalPadding: CGFloat = 4 + private static let buttonFontSize: CGFloat = 11 + + override var isFlipped: Bool { true } + + // MARK: - Init + + init() { + super.init(frame: .zero) + setupViews() + } + + @available(*, unavailable) + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + // MARK: - Setup + + private func setupViews() { + wantsLayer = true + layer?.cornerRadius = Self.cornerRadius + layer?.masksToBounds = true + + // Background: system material (vibrancy) + let bgColor = NSColor(name: nil) { appearance in + if appearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua { + return NSColor.controlBackgroundColor.withAlphaComponent(0.95) + } else { + return NSColor.controlBackgroundColor.withAlphaComponent(0.92) + } + } + layer?.backgroundColor = bgColor.cgColor + + // Border + let borderColor = NSColor(name: nil) { appearance in + if appearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua { + return NSColor.separatorColor.withAlphaComponent(0.5) + } else { + return NSColor.separatorColor.withAlphaComponent(0.3) + } + } + layer?.borderColor = borderColor.cgColor + layer?.borderWidth = 0.5 + + // Shadow + shadow = NSShadow() + layer?.shadowColor = NSColor.black.withAlphaComponent(0.15).cgColor + layer?.shadowOffset = CGSize(width: 0, height: 1) + layer?.shadowRadius = 3 + layer?.shadowOpacity = 1 + + // Configure buttons + configureButton( + prevButton, + symbolName: "chevron.up", + tooltip: "Previous Change", + accessibilityID: HunkToolbarAction.previousHunk.accessibilityID, + action: #selector(prevClicked) + ) + configureButton( + nextButton, + symbolName: "chevron.down", + tooltip: "Next Change", + accessibilityID: HunkToolbarAction.nextHunk.accessibilityID, + action: #selector(nextClicked) + ) + configureButton( + restoreButton, + symbolName: "arrow.uturn.backward", + tooltip: "Restore", + accessibilityID: HunkToolbarAction.restore.accessibilityID, + action: #selector(restoreClicked) + ) + configureButton( + dismissButton, + symbolName: "xmark", + tooltip: "Dismiss", + accessibilityID: HunkToolbarAction.dismiss.accessibilityID, + action: #selector(dismissClicked) + ) + + // Summary label + summaryLabel.font = NSFont.monospacedSystemFont(ofSize: Self.buttonFontSize, weight: .medium) + summaryLabel.textColor = .secondaryLabelColor + summaryLabel.setContentHuggingPriority(.defaultHigh, for: .horizontal) + summaryLabel.setAccessibilityIdentifier("hunk-toolbar-summary") + + // Separator views + let sep1 = makeSeparator() + let sep2 = makeSeparator() + + // Stack + stackView.orientation = .horizontal + stackView.spacing = 2 + stackView.alignment = .centerY + stackView.edgeInsets = NSEdgeInsets( + top: 0, + left: Self.horizontalPadding, + bottom: 0, + right: Self.horizontalPadding + ) + stackView.setViews( + [prevButton, nextButton, sep1, summaryLabel, sep2, restoreButton, dismissButton], + in: .center + ) + + addSubview(stackView) + stackView.translatesAutoresizingMaskIntoConstraints = false + NSLayoutConstraint.activate([ + stackView.leadingAnchor.constraint(equalTo: leadingAnchor), + stackView.trailingAnchor.constraint(equalTo: trailingAnchor), + stackView.topAnchor.constraint(equalTo: topAnchor), + stackView.bottomAnchor.constraint(equalTo: bottomAnchor) + ]) + + setAccessibilityIdentifier("hunk-toolbar") + } + + private func configureButton( + _ button: NSButton, + symbolName: String, + tooltip: String, + accessibilityID: String, + action: Selector + ) { + let config = NSImage.SymbolConfiguration(pointSize: Self.buttonFontSize, weight: .medium) + if let image = NSImage(systemSymbolName: symbolName, accessibilityDescription: tooltip)? + .withSymbolConfiguration(config) { + button.image = image + } + button.isBordered = false + button.bezelStyle = .accessoryBarAction + button.toolTip = tooltip + button.target = self + button.action = action + button.setAccessibilityIdentifier(accessibilityID) + button.setContentHuggingPriority(.defaultHigh, for: .horizontal) + button.setContentHuggingPriority(.defaultHigh, for: .vertical) + } + + private func makeSeparator() -> NSView { + let sep = NSView() + sep.wantsLayer = true + sep.layer?.backgroundColor = NSColor.separatorColor.withAlphaComponent(0.3).cgColor + sep.translatesAutoresizingMaskIntoConstraints = false + NSLayoutConstraint.activate([ + sep.widthAnchor.constraint(equalToConstant: 1), + sep.heightAnchor.constraint(equalToConstant: 14) + ]) + return sep + } + + // MARK: - Actions + + @objc private func prevClicked() { + onAction?(.previousHunk) + } + + @objc private func nextClicked() { + onAction?(.nextHunk) + } + + @objc private func restoreClicked() { + onAction?(.restore) + } + + @objc private func dismissClicked() { + onAction?(.dismiss) + } + + // MARK: - Sizing + + /// Calculates the ideal width for the toolbar based on its contents. + func idealSize() -> NSSize { + stackView.layoutSubtreeIfNeeded() + let fittingSize = stackView.fittingSize + return NSSize( + width: fittingSize.width + Self.horizontalPadding * 2, + height: Self.toolbarHeight + ) + } +} diff --git a/Pine/InlineDiffProvider.swift b/Pine/InlineDiffProvider.swift index dbf0302..4a9f3cd 100644 --- a/Pine/InlineDiffProvider.swift +++ b/Pine/InlineDiffProvider.swift @@ -224,6 +224,56 @@ enum InlineDiffProvider { case next, previous } + // MARK: - Hunk navigation (toolbar) + + /// Returns the next hunk after the given one, wrapping around to the first. + /// Returns nil if the hunks array is empty. + /// If the current hunk is not found (stale), returns the first hunk. + static func nextHunk(after current: DiffHunk, in hunks: [DiffHunk]) -> DiffHunk? { + guard !hunks.isEmpty else { return nil } + guard let index = hunks.firstIndex(where: { $0.id == current.id }) else { + return hunks.first + } + return hunks[(index + 1) % hunks.count] + } + + /// Returns the previous hunk before the given one, wrapping around to the last. + /// Returns nil if the hunks array is empty. + /// If the current hunk is not found (stale), returns the last hunk. + static func previousHunk(before current: DiffHunk, in hunks: [DiffHunk]) -> DiffHunk? { + guard !hunks.isEmpty else { return nil } + guard let index = hunks.firstIndex(where: { $0.id == current.id }) else { + return hunks.last + } + return hunks[(index - 1 + hunks.count) % hunks.count] + } + + /// Returns the 1-based position and total count for a hunk in the list. + /// Returns nil if the hunk is not found. + static func hunkPositionInfo(for hunk: DiffHunk, in hunks: [DiffHunk]) -> (index: Int, total: Int)? { + guard let idx = hunks.firstIndex(where: { $0.id == hunk.id }) else { return nil } + return (idx + 1, hunks.count) + } + + /// Returns a short summary string for a hunk (e.g. "+3 -2" or "+1"). + static func hunkSummary(_ hunk: DiffHunk) -> String { + let added = hunk.addedLines.count + let deleted = hunk.deletedLines.count + var parts: [String] = [] + if added > 0 { parts.append("+\(added)") } + if deleted > 0 { parts.append("-\(deleted)") } + return parts.joined(separator: " ") + } + + /// Returns the line range covered by a hunk in the editor (1-based, inclusive). + /// For pure deletion hunks (newCount == 0), returns just the anchor line. + static func expandedLineRange(for hunk: DiffHunk) -> ClosedRange { + if hunk.newCount == 0 { + return hunk.newStart...hunk.newStart + } + return hunk.newStart...hunk.newEndLine + } + // MARK: - Hunk classification /// Returns `true` when the hunk represents a modification (has both deleted and added lines). diff --git a/PineTests/InlineDiffHunkViewerTests.swift b/PineTests/InlineDiffHunkViewerTests.swift new file mode 100644 index 0000000..61e0f94 --- /dev/null +++ b/PineTests/InlineDiffHunkViewerTests.swift @@ -0,0 +1,427 @@ +// +// InlineDiffHunkViewerTests.swift +// PineTests +// +// Tests for inline diff hunk viewer (#689): +// - Hunk navigation (next/previous) from expanded state +// - Dismiss on click outside hunk area +// - Restore (revert) action from toolbar +// - Toolbar button actions mapping +// + +import Testing +import AppKit +@testable import Pine + +@Suite("Inline Diff Hunk Viewer Tests") +struct InlineDiffHunkViewerTests { + + // MARK: - Helpers + + private func makeHunk( + newStart: Int = 2, + newCount: Int = 2, + oldStart: Int = 2, + oldCount: Int = 1, + rawText: String = "@@ -2,1 +2,2 @@\n context\n+added line\n" + ) -> DiffHunk { + DiffHunk( + newStart: newStart, + newCount: newCount, + oldStart: oldStart, + oldCount: oldCount, + rawText: rawText + ) + } + + private func makeTextView(text: String = "line1\nline2\nline3\nline4\nline5\n") -> GutterTextView { + let textStorage = NSTextStorage(string: text) + let layoutManager = NSLayoutManager() + textStorage.addLayoutManager(layoutManager) + let textContainer = NSTextContainer( + containerSize: NSSize(width: 500, height: CGFloat.greatestFiniteMagnitude) + ) + layoutManager.addTextContainer(textContainer) + return GutterTextView( + frame: NSRect(x: 0, y: 0, width: 500, height: 500), + textContainer: textContainer + ) + } + + // MARK: - Hunk Navigation + + @Test func navigateToNextHunkFromExpanded() { + let hunk1 = makeHunk(newStart: 2, newCount: 2) + let hunk2 = makeHunk(newStart: 8, newCount: 3) + let hunk3 = makeHunk(newStart: 15, newCount: 1) + let hunks = [hunk1, hunk2, hunk3] + + // Currently expanded hunk1, navigate next should return hunk2 + let next = InlineDiffProvider.nextHunk(after: hunk1, in: hunks) + #expect(next?.id == hunk2.id) + } + + @Test func navigateToNextHunkWrapsAround() { + let hunk1 = makeHunk(newStart: 2, newCount: 2) + let hunk2 = makeHunk(newStart: 8, newCount: 3) + let hunks = [hunk1, hunk2] + + // Currently at last hunk, should wrap to first + let next = InlineDiffProvider.nextHunk(after: hunk2, in: hunks) + #expect(next?.id == hunk1.id) + } + + @Test func navigateToPreviousHunkFromExpanded() { + let hunk1 = makeHunk(newStart: 2, newCount: 2) + let hunk2 = makeHunk(newStart: 8, newCount: 3) + let hunk3 = makeHunk(newStart: 15, newCount: 1) + let hunks = [hunk1, hunk2, hunk3] + + // Currently expanded hunk2, navigate previous should return hunk1 + let prev = InlineDiffProvider.previousHunk(before: hunk2, in: hunks) + #expect(prev?.id == hunk1.id) + } + + @Test func navigateToPreviousHunkWrapsAround() { + let hunk1 = makeHunk(newStart: 2, newCount: 2) + let hunk2 = makeHunk(newStart: 8, newCount: 3) + let hunks = [hunk1, hunk2] + + // Currently at first hunk, should wrap to last + let prev = InlineDiffProvider.previousHunk(before: hunk1, in: hunks) + #expect(prev?.id == hunk2.id) + } + + @Test func navigateNextWithSingleHunkReturnsSelf() { + let hunk = makeHunk(newStart: 2, newCount: 2) + let next = InlineDiffProvider.nextHunk(after: hunk, in: [hunk]) + #expect(next?.id == hunk.id) + } + + @Test func navigatePreviousWithSingleHunkReturnsSelf() { + let hunk = makeHunk(newStart: 2, newCount: 2) + let prev = InlineDiffProvider.previousHunk(before: hunk, in: [hunk]) + #expect(prev?.id == hunk.id) + } + + @Test func navigateNextWithEmptyHunksReturnsNil() { + let hunk = makeHunk(newStart: 2, newCount: 2) + let next = InlineDiffProvider.nextHunk(after: hunk, in: []) + #expect(next == nil) + } + + @Test func navigatePreviousWithEmptyHunksReturnsNil() { + let hunk = makeHunk(newStart: 2, newCount: 2) + let prev = InlineDiffProvider.previousHunk(before: hunk, in: []) + #expect(prev == nil) + } + + @Test func navigateNextWithStaleHunkReturnsFirst() { + let hunk1 = makeHunk(newStart: 2, newCount: 2) + let hunk2 = makeHunk(newStart: 8, newCount: 3) + let staleHunk = makeHunk(newStart: 100, newCount: 1) + + // Stale hunk not found in list — should return first hunk + let next = InlineDiffProvider.nextHunk(after: staleHunk, in: [hunk1, hunk2]) + #expect(next?.id == hunk1.id) + } + + @Test func navigatePreviousWithStaleHunkReturnsLast() { + let hunk1 = makeHunk(newStart: 2, newCount: 2) + let hunk2 = makeHunk(newStart: 8, newCount: 3) + let staleHunk = makeHunk(newStart: 100, newCount: 1) + + // Stale hunk not found in list — should return last hunk + let prev = InlineDiffProvider.previousHunk(before: staleHunk, in: [hunk1, hunk2]) + #expect(prev?.id == hunk2.id) + } + + // MARK: - Click Outside Dismisses Hunk + + @Test func clickOutsideHunkDismissesExpanded() { + let tv = makeTextView() + let hunk = makeHunk(newStart: 2, newCount: 2) + tv.diffHunksForHighlight = [hunk] + tv.expandedHunkID = hunk.id + #expect(tv.expandedHunkID != nil) + + // Line 5 is outside hunk range (2-3) + let shouldDismiss = tv.shouldDismissHunkOnClick(atLine: 5) + #expect(shouldDismiss == true) + } + + @Test func clickInsideHunkDoesNotDismiss() { + let tv = makeTextView() + let hunk = makeHunk(newStart: 2, newCount: 2) + tv.diffHunksForHighlight = [hunk] + tv.expandedHunkID = hunk.id + + // Line 2 is inside hunk range (2-3) + let shouldDismiss = tv.shouldDismissHunkOnClick(atLine: 2) + #expect(shouldDismiss == false) + } + + @Test func clickInsideHunkLastLineDoesNotDismiss() { + let tv = makeTextView() + let hunk = makeHunk(newStart: 2, newCount: 3) + tv.diffHunksForHighlight = [hunk] + tv.expandedHunkID = hunk.id + + // Line 4 is inside hunk range (2-4) + let shouldDismiss = tv.shouldDismissHunkOnClick(atLine: 4) + #expect(shouldDismiss == false) + } + + @Test func clickWithNoExpandedHunkDoesNotDismiss() { + let tv = makeTextView() + tv.expandedHunkID = nil + + let shouldDismiss = tv.shouldDismissHunkOnClick(atLine: 3) + #expect(shouldDismiss == false) + } + + @Test func clickOnPureDeletionHunkDoesNotDismiss() { + let tv = makeTextView() + let hunk = DiffHunk( + newStart: 3, newCount: 0, oldStart: 3, oldCount: 2, + rawText: "@@ -3,2 +3,0 @@\n-deleted1\n-deleted2" + ) + tv.diffHunksForHighlight = [hunk] + tv.expandedHunkID = hunk.id + + // Line 3 is the anchor for a pure deletion hunk + let shouldDismiss = tv.shouldDismissHunkOnClick(atLine: 3) + #expect(shouldDismiss == false) + } + + // MARK: - Hunk Position Info + + @Test func hunkPositionInfoCorrect() { + let hunk1 = makeHunk(newStart: 2) + let hunk2 = makeHunk(newStart: 8) + let hunk3 = makeHunk(newStart: 15) + let hunks = [hunk1, hunk2, hunk3] + + let info = InlineDiffProvider.hunkPositionInfo(for: hunk2, in: hunks) + #expect(info?.index == 2) + #expect(info?.total == 3) + } + + @Test func hunkPositionInfoFirstHunk() { + let hunk1 = makeHunk(newStart: 2) + let hunk2 = makeHunk(newStart: 8) + let hunks = [hunk1, hunk2] + + let info = InlineDiffProvider.hunkPositionInfo(for: hunk1, in: hunks) + #expect(info?.index == 1) + #expect(info?.total == 2) + } + + @Test func hunkPositionInfoSingleHunk() { + let hunk = makeHunk(newStart: 5) + let info = InlineDiffProvider.hunkPositionInfo(for: hunk, in: [hunk]) + #expect(info?.index == 1) + #expect(info?.total == 1) + } + + @Test func hunkPositionInfoStaleHunkReturnsNil() { + let hunk1 = makeHunk(newStart: 2) + let staleHunk = makeHunk(newStart: 100) + let info = InlineDiffProvider.hunkPositionInfo(for: staleHunk, in: [hunk1]) + #expect(info == nil) + } + + // MARK: - Toolbar Action Enum + + @Test func hunkToolbarActionValues() { + let actions: [HunkToolbarAction] = [.previousHunk, .nextHunk, .restore, .dismiss] + #expect(actions.count == 4) + } + + @Test func hunkToolbarActionAccessibilityIDs() { + #expect(HunkToolbarAction.previousHunk.accessibilityID == "hunk-toolbar-previous") + #expect(HunkToolbarAction.nextHunk.accessibilityID == "hunk-toolbar-next") + #expect(HunkToolbarAction.restore.accessibilityID == "hunk-toolbar-restore") + #expect(HunkToolbarAction.dismiss.accessibilityID == "hunk-toolbar-dismiss") + } + + // MARK: - Hunk Summary Text + + @Test func hunkSummaryForAddition() { + let hunk = DiffHunk( + newStart: 5, newCount: 3, oldStart: 5, oldCount: 0, + rawText: "@@ -5,0 +5,3 @@\n+line1\n+line2\n+line3" + ) + let summary = InlineDiffProvider.hunkSummary(hunk) + #expect(summary.contains("+3")) + #expect(!summary.contains("-")) + } + + @Test func hunkSummaryForDeletion() { + let hunk = DiffHunk( + newStart: 5, newCount: 0, oldStart: 5, oldCount: 2, + rawText: "@@ -5,2 +5,0 @@\n-line1\n-line2" + ) + let summary = InlineDiffProvider.hunkSummary(hunk) + #expect(summary.contains("-2")) + } + + @Test func hunkSummaryForModification() { + let hunk = DiffHunk( + newStart: 5, newCount: 3, oldStart: 5, oldCount: 2, + rawText: "@@ -5,2 +5,3 @@\n-old1\n-old2\n+new1\n+new2\n+new3" + ) + let summary = InlineDiffProvider.hunkSummary(hunk) + #expect(summary.contains("+3")) + #expect(summary.contains("-2")) + } + + // MARK: - Expanded hunk line range helper + + @Test func expandedHunkLineRangeForNormalHunk() { + let hunk = makeHunk(newStart: 5, newCount: 3) + let range = InlineDiffProvider.expandedLineRange(for: hunk) + #expect(range == 5...7) + } + + @Test func expandedHunkLineRangeForPureDeletion() { + let hunk = DiffHunk( + newStart: 5, newCount: 0, oldStart: 5, oldCount: 3, + rawText: "@@ -5,3 +5,0 @@\n-a\n-b\n-c" + ) + let range = InlineDiffProvider.expandedLineRange(for: hunk) + #expect(range == 5...5) + } + + @Test func expandedHunkLineRangeForSingleLine() { + let hunk = makeHunk(newStart: 10, newCount: 1) + let range = InlineDiffProvider.expandedLineRange(for: hunk) + #expect(range == 10...10) + } + + // MARK: - onHunkDismissed callback + + @Test func onHunkDismissedCalledWhenExpandedHunkSetToNil() { + let tv = makeTextView() + var dismissed = false + tv.onHunkDismissed = { dismissed = true } + + tv.expandedHunkID = UUID() + #expect(!dismissed) + + tv.expandedHunkID = nil + #expect(dismissed) + } + + @Test func onHunkDismissedNotCalledWhenSettingNewHunkID() { + let tv = makeTextView() + var dismissCount = 0 + tv.onHunkDismissed = { dismissCount += 1 } + + tv.expandedHunkID = UUID() + tv.expandedHunkID = UUID() // switching to different hunk + #expect(dismissCount == 0) + } + + @Test func onHunkDismissedCalledFromDismissExpandedHunk() { + let tv = makeTextView() + var dismissed = false + tv.onHunkDismissed = { dismissed = true } + + tv.expandedHunkID = UUID() + tv.dismissExpandedHunk() + #expect(dismissed) + } + + // MARK: - HunkToolbarView basic creation + + @Test func hunkToolbarViewCreation() { + let toolbar = HunkToolbarView() + #expect(toolbar.accessibilityIdentifier() == "hunk-toolbar") + } + + @Test func hunkToolbarViewSummaryText() { + let toolbar = HunkToolbarView() + toolbar.summaryText = "2/5 +3 -1" + // Verify it doesn't crash and the frame is non-zero after layout + let size = toolbar.idealSize() + #expect(size.width > 0) + #expect(size.height > 0) + } + + @Test func hunkToolbarViewActionCallback() { + let toolbar = HunkToolbarView() + var receivedAction: HunkToolbarAction? + toolbar.onAction = { action in + receivedAction = action + } + // Simulate action via callback + toolbar.onAction?(.restore) + #expect(receivedAction == .restore) + } + + // MARK: - Hunk summary edge cases + + @Test func hunkSummaryForContextOnlyHunk() { + // A hunk that has no added or deleted lines (shouldn't happen in practice, but test it) + let hunk = DiffHunk( + newStart: 1, newCount: 3, oldStart: 1, oldCount: 3, + rawText: "@@ -1,3 +1,3 @@\n context1\n context2\n context3" + ) + let summary = InlineDiffProvider.hunkSummary(hunk) + #expect(summary.isEmpty) + } + + // MARK: - Navigation with many hunks + + @Test func navigateNextThroughAllHunks() { + let hunks = (0..<5).map { makeHunk(newStart: $0 * 10 + 1) } + + var current = hunks[0] + for i in 1..<5 { + guard let next = InlineDiffProvider.nextHunk(after: current, in: hunks) else { + Issue.record("nextHunk returned nil at index \(i)") + return + } + #expect(next.id == hunks[i].id) + current = next + } + // Wrap around + let wrapped = InlineDiffProvider.nextHunk(after: current, in: hunks) + #expect(wrapped?.id == hunks[0].id) + } + + @Test func navigatePreviousThroughAllHunks() { + let hunks = (0..<5).map { makeHunk(newStart: $0 * 10 + 1) } + + var current = hunks[4] + for i in stride(from: 3, through: 0, by: -1) { + guard let prev = InlineDiffProvider.previousHunk(before: current, in: hunks) else { + Issue.record("previousHunk returned nil at index \(i)") + return + } + #expect(prev.id == hunks[i].id) + current = prev + } + // Wrap around + let wrapped = InlineDiffProvider.previousHunk(before: current, in: hunks) + #expect(wrapped?.id == hunks[4].id) + } + + // MARK: - Click outside with multiple hunks + + @Test func clickOutsideMultipleHunksDismisses() { + let tv = makeTextView(text: String(repeating: "line\n", count: 30)) + let hunk1 = makeHunk(newStart: 2, newCount: 3) + let hunk2 = makeHunk(newStart: 10, newCount: 2) + tv.diffHunksForHighlight = [hunk1, hunk2] + tv.expandedHunkID = hunk1.id + + // Line 7 is between hunk1 (2-4) and hunk2 (10-11) + #expect(tv.shouldDismissHunkOnClick(atLine: 7) == true) + // Line 10 is inside hunk2 but hunk1 is expanded, so should dismiss + #expect(tv.shouldDismissHunkOnClick(atLine: 10) == true) + // Line 3 is inside hunk1 (expanded), should not dismiss + #expect(tv.shouldDismissHunkOnClick(atLine: 3) == false) + } +} From 17e458e583dce37e0e7c680b36b8b505bd8bf841 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 06:42:39 +0300 Subject: [PATCH 2/4] =?UTF-8?q?fix:=20address=20PR=20#691=20review=20?= =?UTF-8?q?=E2=80=94=20shadow,=20appearance,=20perf,=20strings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove masksToBounds that clipped shadow on HunkToolbarView - Add viewDidChangeEffectiveAppearance to update colors on Dark/Light switch - Use LineStartsCache in lineNumberAtPoint for O(log n) lookup - Extract duplicated line-to-charOffset while-loop into shared helper - Move toolbar accessibility IDs to AccessibilityIdentifiers.swift - Extract hardcoded tooltip strings to Strings.swift with localizations --- Pine/AccessibilityIdentifiers.swift | 8 +++ Pine/CodeEditorView.swift | 60 ++++++++++++------- Pine/HunkToolbarAction.swift | 7 ++- Pine/HunkToolbarView.swift | 69 +++++++++++----------- Pine/LineStartsCache.swift | 7 +++ Pine/Localizable.xcstrings | 72 +++++++++++++++++++++++ Pine/Strings.swift | 18 ++++++ PineTests/InlineDiffHunkViewerTests.swift | 26 ++++++-- PineTests/LineStartsCacheTests.swift | 49 +++++++++++++++ 9 files changed, 255 insertions(+), 61 deletions(-) diff --git a/Pine/AccessibilityIdentifiers.swift b/Pine/AccessibilityIdentifiers.swift index e9de017..1503b00 100644 --- a/Pine/AccessibilityIdentifiers.swift +++ b/Pine/AccessibilityIdentifiers.swift @@ -96,6 +96,14 @@ nonisolated enum AccessibilityID { static let symbolResultsList = "symbolResultsList" static func symbolItem(_ name: String) -> String { "symbolItem_\(name)" } + // MARK: - Hunk Toolbar + static let hunkToolbar = "hunk-toolbar" + static let hunkToolbarPrevious = "hunk-toolbar-previous" + static let hunkToolbarNext = "hunk-toolbar-next" + static let hunkToolbarRestore = "hunk-toolbar-restore" + static let hunkToolbarDismiss = "hunk-toolbar-dismiss" + static let hunkToolbarSummary = "hunk-toolbar-summary" + // MARK: - Toast notifications static let toastNotification = "toastNotification" diff --git a/Pine/CodeEditorView.swift b/Pine/CodeEditorView.swift index da354dd..9f89b42 100644 --- a/Pine/CodeEditorView.swift +++ b/Pine/CodeEditorView.swift @@ -62,6 +62,9 @@ final class GutterTextView: NSTextView { /// Callback invoked when the expanded hunk is dismissed (for toolbar cleanup). var onHunkDismissed: (() -> Void)? + /// Cached line starts for O(log n) line lookups (set by Coordinator). + var lineStartsCache: LineStartsCache? + /// Subtle green tint for added lines. private static let addedLineColor = NSColor(name: nil) { appearance in if appearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua { @@ -524,6 +527,7 @@ final class GutterTextView: NSTextView { } /// Returns the 1-based line number at the given point in text view coordinates. + /// Uses `lineStartsCache` for O(log n) lookup when available. private func lineNumberAtPoint(_ point: NSPoint) -> Int? { guard let layoutManager = layoutManager, let textContainer = textContainer else { return nil } @@ -535,6 +539,11 @@ final class GutterTextView: NSTextView { let charIndex = layoutManager.characterIndexForGlyph(at: glyphIndex) let source = string as NSString guard charIndex <= source.length else { return nil } + + if let cache = lineStartsCache { + return cache.lineNumber(at: charIndex) + } + // Fallback: O(n) scan if cache is unavailable var line = 1 for i in 0.. 0 else { return } // Find the character offset for the hunk's start line - let targetLine = hunk.newStart - var charOffset = 0 - var currentLine = 1 - while currentLine < targetLine && charOffset < source.length { - if source.character(at: charOffset) == ASCII.newline { - currentLine += 1 - } - charOffset += 1 - } + let charOffset = CodeEditorView.charOffsetForLine( + hunk.newStart, in: source, totalLength: source.length, cache: lineStartsCache + ) let glyphIndex = layoutManager.glyphIndexForCharacter(at: min(charOffset, source.length - 1)) let lineRect = layoutManager.lineFragmentRect(forGlyphAt: glyphIndex, effectiveRange: nil) @@ -2034,14 +2044,9 @@ struct CodeEditorView: NSViewRepresentable { guard source.length > 0, let layoutManager = textView.layoutManager else { return } - var charOffset = 0 - var currentLine = 1 - while currentLine < hunk.newStart && charOffset < source.length { - if source.character(at: charOffset) == ASCII.newline { - currentLine += 1 - } - charOffset += 1 - } + let charOffset = CodeEditorView.charOffsetForLine( + hunk.newStart, in: source, totalLength: source.length, cache: lineStartsCache + ) let glyphIndex = layoutManager.glyphIndexForCharacter(at: min(charOffset, source.length - 1)) let lineRect = layoutManager.lineFragmentRect(forGlyphAt: glyphIndex, effectiveRange: nil) @@ -2236,13 +2241,13 @@ struct CodeEditorView: NSViewRepresentable { if scrollOffset > 0 { let lineHeight = fontSize * 1.2 let estimatedLine = Int(scrollOffset / lineHeight) - startChar = charOffsetForLine(estimatedLine, in: source, totalLength: totalLength) + startChar = charOffsetForLine(estimatedLine + 1, in: source, totalLength: totalLength) } else if cursorPosition > 0 && cursorPosition < totalLength { // Center around cursor let linesBefore = estimatedScreenLines / 2 let cursorLine = lineNumber(at: cursorPosition, in: source) let startLine = max(0, cursorLine - linesBefore) - startChar = charOffsetForLine(startLine, in: source, totalLength: totalLength) + startChar = charOffsetForLine(startLine + 1, in: source, totalLength: totalLength) } else { startChar = 0 } @@ -2258,8 +2263,19 @@ struct CodeEditorView: NSViewRepresentable { return NSRange(location: startChar, length: end - startChar) } - private static func charOffsetForLine(_ line: Int, in source: NSString, totalLength: Int) -> Int { - var currentLine = 0 + /// Returns the UTF-16 character offset for the start of a 1-based line number. + /// Uses `LineStartsCache` for O(log n) when available, falls back to O(n) scan. + static func charOffsetForLine( + _ line: Int, + in source: NSString, + totalLength: Int, + cache: LineStartsCache? = nil + ) -> Int { + if let cache { + return cache.charOffset(forLine: line) + } + // Fallback: O(n) linear scan — 1-based line number + var currentLine = 1 for i in 0..= line { return i } if source.character(at: i) == ASCII.newline { currentLine += 1 } diff --git a/Pine/HunkToolbarAction.swift b/Pine/HunkToolbarAction.swift index ade0603..a70c9be 100644 --- a/Pine/HunkToolbarAction.swift +++ b/Pine/HunkToolbarAction.swift @@ -16,6 +16,11 @@ enum HunkToolbarAction: String, Sendable { /// Accessibility identifier for UI testing. var accessibilityID: String { - "hunk-toolbar-\(rawValue.replacingOccurrences(of: "Hunk", with: "").lowercased())" + switch self { + case .previousHunk: return AccessibilityID.hunkToolbarPrevious + case .nextHunk: return AccessibilityID.hunkToolbarNext + case .restore: return AccessibilityID.hunkToolbarRestore + case .dismiss: return AccessibilityID.hunkToolbarDismiss + } } } diff --git a/Pine/HunkToolbarView.swift b/Pine/HunkToolbarView.swift index 2fc318a..4f9caa2 100644 --- a/Pine/HunkToolbarView.swift +++ b/Pine/HunkToolbarView.swift @@ -58,63 +58,44 @@ final class HunkToolbarView: NSView { private func setupViews() { wantsLayer = true layer?.cornerRadius = Self.cornerRadius - layer?.masksToBounds = true - - // Background: system material (vibrancy) - let bgColor = NSColor(name: nil) { appearance in - if appearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua { - return NSColor.controlBackgroundColor.withAlphaComponent(0.95) - } else { - return NSColor.controlBackgroundColor.withAlphaComponent(0.92) - } - } - layer?.backgroundColor = bgColor.cgColor - // Border - let borderColor = NSColor(name: nil) { appearance in - if appearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua { - return NSColor.separatorColor.withAlphaComponent(0.5) - } else { - return NSColor.separatorColor.withAlphaComponent(0.3) - } - } - layer?.borderColor = borderColor.cgColor - layer?.borderWidth = 0.5 - - // Shadow + // Shadow (must not use masksToBounds, otherwise shadow is clipped) shadow = NSShadow() layer?.shadowColor = NSColor.black.withAlphaComponent(0.15).cgColor layer?.shadowOffset = CGSize(width: 0, height: 1) layer?.shadowRadius = 3 layer?.shadowOpacity = 1 + // Apply appearance-dependent colors + updateAppearanceColors() + // Configure buttons configureButton( prevButton, symbolName: "chevron.up", - tooltip: "Previous Change", - accessibilityID: HunkToolbarAction.previousHunk.accessibilityID, + tooltip: Strings.hunkToolbarPreviousChange, + accessibilityID: AccessibilityID.hunkToolbarPrevious, action: #selector(prevClicked) ) configureButton( nextButton, symbolName: "chevron.down", - tooltip: "Next Change", - accessibilityID: HunkToolbarAction.nextHunk.accessibilityID, + tooltip: Strings.hunkToolbarNextChange, + accessibilityID: AccessibilityID.hunkToolbarNext, action: #selector(nextClicked) ) configureButton( restoreButton, symbolName: "arrow.uturn.backward", - tooltip: "Restore", - accessibilityID: HunkToolbarAction.restore.accessibilityID, + tooltip: Strings.hunkToolbarRestore, + accessibilityID: AccessibilityID.hunkToolbarRestore, action: #selector(restoreClicked) ) configureButton( dismissButton, symbolName: "xmark", - tooltip: "Dismiss", - accessibilityID: HunkToolbarAction.dismiss.accessibilityID, + tooltip: Strings.hunkToolbarDismiss, + accessibilityID: AccessibilityID.hunkToolbarDismiss, action: #selector(dismissClicked) ) @@ -122,7 +103,7 @@ final class HunkToolbarView: NSView { summaryLabel.font = NSFont.monospacedSystemFont(ofSize: Self.buttonFontSize, weight: .medium) summaryLabel.textColor = .secondaryLabelColor summaryLabel.setContentHuggingPriority(.defaultHigh, for: .horizontal) - summaryLabel.setAccessibilityIdentifier("hunk-toolbar-summary") + summaryLabel.setAccessibilityIdentifier(AccessibilityID.hunkToolbarSummary) // Separator views let sep1 = makeSeparator() @@ -152,7 +133,7 @@ final class HunkToolbarView: NSView { stackView.bottomAnchor.constraint(equalTo: bottomAnchor) ]) - setAccessibilityIdentifier("hunk-toolbar") + setAccessibilityIdentifier(AccessibilityID.hunkToolbar) } private func configureButton( @@ -207,6 +188,28 @@ final class HunkToolbarView: NSView { onAction?(.dismiss) } + // MARK: - Appearance Updates + + override func viewDidChangeEffectiveAppearance() { + super.viewDidChangeEffectiveAppearance() + updateAppearanceColors() + } + + /// Recomputes background and border colors for the current appearance (Dark/Light mode). + private func updateAppearanceColors() { + let isDark = effectiveAppearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua + let bgColor = isDark + ? NSColor.controlBackgroundColor.withAlphaComponent(0.95) + : NSColor.controlBackgroundColor.withAlphaComponent(0.92) + layer?.backgroundColor = bgColor.cgColor + + let border = isDark + ? NSColor.separatorColor.withAlphaComponent(0.5) + : NSColor.separatorColor.withAlphaComponent(0.3) + layer?.borderColor = border.cgColor + layer?.borderWidth = 0.5 + } + // MARK: - Sizing /// Calculates the ideal width for the toolbar based on its contents. diff --git a/Pine/LineStartsCache.swift b/Pine/LineStartsCache.swift index bfcab79..fb099f7 100644 --- a/Pine/LineStartsCache.swift +++ b/Pine/LineStartsCache.swift @@ -50,6 +50,13 @@ struct LineStartsCache { lineIndex(containing: charIndex) + 1 } + /// Возвращает UTF-16 символьное смещение начала строки с данным 1-based номером. + /// Для строк за пределами кэша возвращает смещение последней строки. + func charOffset(forLine line: Int) -> Int { + let index = max(0, min(line - 1, lineStarts.count - 1)) + return lineStarts[index] + } + /// Инкрементально обновляет кэш после редактирования текста. /// - Parameters: /// - editedRange: Диапазон в новом тексте, покрывающий вставленный/изменённый контент. diff --git a/Pine/Localizable.xcstrings b/Pine/Localizable.xcstrings index 4de42b2..7d31502 100644 --- a/Pine/Localizable.xcstrings +++ b/Pine/Localizable.xcstrings @@ -2463,6 +2463,78 @@ } } }, + "hunkToolbar.dismiss" : { + "comment" : "Tooltip for dismiss button in the inline diff hunk toolbar.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Dismiss" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Закрыть" + } + } + } + }, + "hunkToolbar.nextChange" : { + "comment" : "Tooltip for next change button in the inline diff hunk toolbar.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Next Change" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Следующее изменение" + } + } + } + }, + "hunkToolbar.previousChange" : { + "comment" : "Tooltip for previous change button in the inline diff hunk toolbar.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Previous Change" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Предыдущее изменение" + } + } + } + }, + "hunkToolbar.restore" : { + "comment" : "Tooltip for restore (revert) button in the inline diff hunk toolbar.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Restore" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Восстановить" + } + } + } + }, "largeFile.openWithHighlighting" : { "comment" : "Button: open a large file with syntax highlighting enabled.", "extractionState" : "manual", diff --git a/Pine/Strings.swift b/Pine/Strings.swift index 2b2477e..5f92dd0 100644 --- a/Pine/Strings.swift +++ b/Pine/Strings.swift @@ -425,6 +425,24 @@ enum Strings { String(localized: "toast.filesReloaded.more \(count) \(names) \(remaining)") } + // MARK: - Hunk Toolbar + + static var hunkToolbarPreviousChange: String { + String(localized: "hunkToolbar.previousChange") + } + + static var hunkToolbarNextChange: String { + String(localized: "hunkToolbar.nextChange") + } + + static var hunkToolbarRestore: String { + String(localized: "hunkToolbar.restore") + } + + static var hunkToolbarDismiss: String { + String(localized: "hunkToolbar.dismiss") + } + // MARK: - Progress Indicators static var progressLoadingProject: String { diff --git a/PineTests/InlineDiffHunkViewerTests.swift b/PineTests/InlineDiffHunkViewerTests.swift index 61e0f94..5927702 100644 --- a/PineTests/InlineDiffHunkViewerTests.swift +++ b/PineTests/InlineDiffHunkViewerTests.swift @@ -239,10 +239,10 @@ struct InlineDiffHunkViewerTests { } @Test func hunkToolbarActionAccessibilityIDs() { - #expect(HunkToolbarAction.previousHunk.accessibilityID == "hunk-toolbar-previous") - #expect(HunkToolbarAction.nextHunk.accessibilityID == "hunk-toolbar-next") - #expect(HunkToolbarAction.restore.accessibilityID == "hunk-toolbar-restore") - #expect(HunkToolbarAction.dismiss.accessibilityID == "hunk-toolbar-dismiss") + #expect(HunkToolbarAction.previousHunk.accessibilityID == AccessibilityID.hunkToolbarPrevious) + #expect(HunkToolbarAction.nextHunk.accessibilityID == AccessibilityID.hunkToolbarNext) + #expect(HunkToolbarAction.restore.accessibilityID == AccessibilityID.hunkToolbarRestore) + #expect(HunkToolbarAction.dismiss.accessibilityID == AccessibilityID.hunkToolbarDismiss) } // MARK: - Hunk Summary Text @@ -337,7 +337,23 @@ struct InlineDiffHunkViewerTests { @Test func hunkToolbarViewCreation() { let toolbar = HunkToolbarView() - #expect(toolbar.accessibilityIdentifier() == "hunk-toolbar") + #expect(toolbar.accessibilityIdentifier() == AccessibilityID.hunkToolbar) + } + + @Test func hunkToolbarViewShadowNotClipped() { + let toolbar = HunkToolbarView() + // masksToBounds must be false so shadow is visible + #expect(toolbar.layer?.masksToBounds != true) + #expect(toolbar.layer?.shadowOpacity == 1) + #expect(toolbar.layer?.shadowRadius == 3) + } + + @Test func hunkToolbarViewHasAppearanceColors() { + let toolbar = HunkToolbarView() + // Background and border should be set after init + #expect(toolbar.layer?.backgroundColor != nil) + #expect(toolbar.layer?.borderColor != nil) + #expect(toolbar.layer?.borderWidth == 0.5) } @Test func hunkToolbarViewSummaryText() { diff --git a/PineTests/LineStartsCacheTests.swift b/PineTests/LineStartsCacheTests.swift index e2db257..bc92993 100644 --- a/PineTests/LineStartsCacheTests.swift +++ b/PineTests/LineStartsCacheTests.swift @@ -214,6 +214,55 @@ struct LineStartsCacheTests { #expect(cache.lineCount == fresh.lineCount) } + // MARK: - charOffset(forLine:) + + @Test func charOffsetForFirstLine() { + let cache = LineStartsCache(text: "abc\ndef\nghi") + #expect(cache.charOffset(forLine: 1) == 0) + } + + @Test func charOffsetForSecondLine() { + let cache = LineStartsCache(text: "abc\ndef\nghi") + #expect(cache.charOffset(forLine: 2) == 4) + } + + @Test func charOffsetForThirdLine() { + let cache = LineStartsCache(text: "abc\ndef\nghi") + #expect(cache.charOffset(forLine: 3) == 8) + } + + @Test func charOffsetForLineBeyondEnd() { + let cache = LineStartsCache(text: "abc\ndef") + // Line 100 is beyond the file — should clamp to last line start + #expect(cache.charOffset(forLine: 100) == 4) + } + + @Test func charOffsetForLineZero() { + let cache = LineStartsCache(text: "abc\ndef") + // Line 0 (invalid, below minimum) — should clamp to first line + #expect(cache.charOffset(forLine: 0) == 0) + } + + @Test func charOffsetForEmptyText() { + let cache = LineStartsCache(text: "") + #expect(cache.charOffset(forLine: 1) == 0) + } + + @Test func charOffsetForSingleLine() { + let cache = LineStartsCache(text: "hello") + #expect(cache.charOffset(forLine: 1) == 0) + #expect(cache.charOffset(forLine: 2) == 0) // clamped to last + } + + @Test func charOffsetRoundTrip() { + // Verify charOffset(forLine:) and lineNumber(at:) are consistent + let cache = LineStartsCache(text: "aaa\nbbb\nccc\nddd\neee") + for line in 1...5 { + let offset = cache.charOffset(forLine: line) + #expect(cache.lineNumber(at: offset) == line) + } + } + // MARK: - Performance characteristic — binary search @Test func largeTextPerformance() { From c2cf0f58f557b3beb42c4273f4a85e834843c98b Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 06:50:45 +0300 Subject: [PATCH 3/4] fix: update separator colors on appearance change in HunkToolbarView Store separator view references as class properties and update their background color in updateAppearanceColors() so Dark/Light mode transitions are reflected correctly. Also remove duplicate MARK comment in CodeEditorView.swift and add test for separator color updates. --- Pine/CodeEditorView.swift | 2 -- Pine/HunkToolbarView.swift | 7 +++++++ PineTests/InlineDiffHunkViewerTests.swift | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Pine/CodeEditorView.swift b/Pine/CodeEditorView.swift index 9f89b42..61b4739 100644 --- a/Pine/CodeEditorView.swift +++ b/Pine/CodeEditorView.swift @@ -562,8 +562,6 @@ final class GutterTextView: NSTextView { super.keyDown(with: event) } - // MARK: - Click outside hunk dismisses expanded inline diff - /// Returns `true` if clicking at the given editor line should dismiss the expanded hunk. /// Returns `false` if no hunk is expanded or the click is inside the expanded hunk's range. func shouldDismissHunkOnClick(atLine line: Int) -> Bool { diff --git a/Pine/HunkToolbarView.swift b/Pine/HunkToolbarView.swift index 4f9caa2..93d8eac 100644 --- a/Pine/HunkToolbarView.swift +++ b/Pine/HunkToolbarView.swift @@ -31,6 +31,7 @@ final class HunkToolbarView: NSView { private let restoreButton = NSButton() private let dismissButton = NSButton() private let stackView = NSStackView() + private(set) var separatorViews: [NSView] = [] // MARK: - Constants @@ -108,6 +109,7 @@ final class HunkToolbarView: NSView { // Separator views let sep1 = makeSeparator() let sep2 = makeSeparator() + separatorViews = [sep1, sep2] // Stack stackView.orientation = .horizontal @@ -208,6 +210,11 @@ final class HunkToolbarView: NSView { : NSColor.separatorColor.withAlphaComponent(0.3) layer?.borderColor = border.cgColor layer?.borderWidth = 0.5 + + let separatorColor = NSColor.separatorColor.withAlphaComponent(0.3).cgColor + for sep in separatorViews { + sep.layer?.backgroundColor = separatorColor + } } // MARK: - Sizing diff --git a/PineTests/InlineDiffHunkViewerTests.swift b/PineTests/InlineDiffHunkViewerTests.swift index 5927702..9b7066c 100644 --- a/PineTests/InlineDiffHunkViewerTests.swift +++ b/PineTests/InlineDiffHunkViewerTests.swift @@ -356,6 +356,20 @@ struct InlineDiffHunkViewerTests { #expect(toolbar.layer?.borderWidth == 0.5) } + @Test func hunkToolbarSeparatorsUpdateWithAppearance() { + let toolbar = HunkToolbarView() + // Separator views should be created and have background color + #expect(toolbar.separatorViews.count == 2) + for sep in toolbar.separatorViews { + #expect(sep.layer?.backgroundColor != nil) + } + // Trigger appearance update — separator colors should still be set + toolbar.viewDidChangeEffectiveAppearance() + for sep in toolbar.separatorViews { + #expect(sep.layer?.backgroundColor != nil) + } + } + @Test func hunkToolbarViewSummaryText() { let toolbar = HunkToolbarView() toolbar.summaryText = "2/5 +3 -1" From 3cd91ad08a2c375cf3c2a4473cf790d93c226e84 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 10:48:39 +0300 Subject: [PATCH 4/4] fix: prevent gutter corruption and layout overlap on expanded hunk (#697, #698) - Skip line number drawing on hunk start line when action buttons are shown (#697) - Exclude HunkToolbarView from EditorContainerView.layout() repositioning (#698) - Add tests for both fixes --- Pine/CodeEditorView.swift | 3 ++- Pine/LineNumberGutter.swift | 28 +++++++++++++------- PineTests/EditorContainerViewTests.swift | 33 ++++++++++++++++++++++++ PineTests/LineNumberViewTests.swift | 22 ++++++++++++++++ 4 files changed, 75 insertions(+), 11 deletions(-) diff --git a/Pine/CodeEditorView.swift b/Pine/CodeEditorView.swift index 61b4739..daff42b 100644 --- a/Pine/CodeEditorView.swift +++ b/Pine/CodeEditorView.swift @@ -665,7 +665,7 @@ final class EditorContainerView: NSView { width: bounds.width - minimapWidth, height: bounds.height ) - } else { + } else if sub is LineNumberView { // LineNumberView — offset below the find bar when Cmd+F is open. sub.frame = NSRect( x: 0, y: findBarOffset, @@ -673,6 +673,7 @@ final class EditorContainerView: NSView { height: bounds.height - findBarOffset ) } + // HunkToolbarView and other overlays manage their own frames — skip them. } } } diff --git a/Pine/LineNumberGutter.swift b/Pine/LineNumberGutter.swift index 9ba36e1..cee99b7 100644 --- a/Pine/LineNumberGutter.swift +++ b/Pine/LineNumberGutter.swift @@ -462,10 +462,24 @@ final class LineNumberView: NSView { // Y: позиция фрагмента в textContainer + сдвиг контейнера − скролл let y = lineRect.origin.y + originY - visibleRect.origin.y - let numStr = "\(lineNumber)" as NSString - let size = numStr.size(withAttributes: attrs) - let x = self.gutterWidth - size.width - 8 - numStr.draw(at: NSPoint(x: x, y: y + self.baselineOffset), withAttributes: attrs) + // Check if this line should show hunk action buttons instead of line number + let isHunkActionLine: Bool + if let hunk = self.hunkStartMap[lineNumber], + self.expandedHunkID == hunk.id { + isHunkActionLine = true + } else { + isHunkActionLine = false + } + + if isHunkActionLine { + // ── Accept/Revert buttons replace the line number on hunk start lines ── + self.drawHunkActionButtons(at: y, lineHeight: lineRect.height) + } else { + let numStr = "\(lineNumber)" as NSString + let size = numStr.size(withAttributes: attrs) + let x = self.gutterWidth - size.width - 8 + numStr.draw(at: NSPoint(x: x, y: y + self.baselineOffset), withAttributes: attrs) + } // ── Fold disclosure triangle ── if showFoldIndicators || self.foldState.foldedRanges.contains(where: { $0.startLine == lineNumber }) { @@ -520,12 +534,6 @@ final class LineNumberView: NSView { ) } - // ── Accept/Revert buttons on hunk start lines (visible when hunk is expanded) ── - if let hunk = self.hunkStartMap[lineNumber], - self.expandedHunkID == hunk.id { - self.drawHunkActionButtons(at: y, lineHeight: lineRect.height) - } - lineNumber += 1 } diff --git a/PineTests/EditorContainerViewTests.swift b/PineTests/EditorContainerViewTests.swift index bd73fac..8c53697 100644 --- a/PineTests/EditorContainerViewTests.swift +++ b/PineTests/EditorContainerViewTests.swift @@ -65,6 +65,39 @@ struct EditorContainerViewTests { #expect(lineNumberView.frame.height == 600) } + // MARK: - HunkToolbarView is not repositioned by layout (#698) + + @Test func layoutDoesNotOverrideHunkToolbarFrame() { + let container = EditorContainerView(frame: NSRect(x: 0, y: 0, width: 1000, height: 600)) + container.minimapWidth = 0 + + let scrollView = EditorScrollView(frame: .zero) + container.addSubview(scrollView) + + let textView = NSTextView(frame: .zero) + let lineNumberView = LineNumberView(textView: textView) + lineNumberView.frame = NSRect(x: 0, y: 0, width: 40, height: 600) + container.addSubview(lineNumberView) + + // Add a HunkToolbarView with a specific frame (as showHunkToolbar would) + let toolbar = HunkToolbarView() + let toolbarFrame = NSRect(x: 800, y: 50, width: 180, height: 24) + toolbar.frame = toolbarFrame + container.addSubview(toolbar) + + container.layout() + + // Toolbar frame must be preserved — layout should not reposition it + #expect(toolbar.frame.origin.x == 800) + #expect(toolbar.frame.origin.y == 50) + #expect(toolbar.frame.size.width == 180) + #expect(toolbar.frame.size.height == 24) + + // LineNumberView should still be positioned correctly + #expect(lineNumberView.frame.origin.x == 0) + #expect(lineNumberView.frame.width == 40) + } + // MARK: - EditorScrollView @Test func tileWithNoFindBarKeepsZeroOffset() { diff --git a/PineTests/LineNumberViewTests.swift b/PineTests/LineNumberViewTests.swift index 321b8e9..c6fafcd 100644 --- a/PineTests/LineNumberViewTests.swift +++ b/PineTests/LineNumberViewTests.swift @@ -111,4 +111,26 @@ struct LineNumberViewTests { #expect(toggledRange?.startLine == 1) #expect(toggledRange?.endLine == 5) } + + // MARK: - Expanded hunk replaces line number (#697) + + @Test func expandedHunkSetsStateOnLineNumberView() { + let (view, _) = makeView() + let hunk = DiffHunk( + newStart: 2, newCount: 3, oldStart: 2, oldCount: 1, + rawText: "@@ -2,1 +2,3 @@\n context\n+added\n+added2\n" + ) + view.diffHunks = [hunk] + view.expandedHunkID = hunk.id + + // When a hunk is expanded, the LineNumberView should know about it + #expect(view.expandedHunkID == hunk.id) + #expect(view.diffHunks.count == 1) + #expect(view.diffHunks.first?.newStart == 2) + } + + @Test func expandedHunkIDNilByDefault() { + let (view, _) = makeView() + #expect(view.expandedHunkID == nil) + } }