feat: expandable inline diff hunk viewer with toolbar

[batonogov]

Summary

• Adds a compact floating toolbar overlay (HunkToolbarView) above expanded diff hunks with navigation arrows (prev/next hunk), restore button, summary label (e.g. "2/5 +3 -1"), and dismiss button
• Click outside the expanded hunk area now dismisses it (was only Escape before)
• Hunk toolbar repositions on scroll to stay aligned with the expanded hunk
• onHunkDismissed callback ensures toolbar cleanup from all dismiss sources (Escape, click outside, diff data change)
• New InlineDiffProvider methods: nextHunk, previousHunk, hunkPositionInfo, hunkSummary, expandedLineRange

Closes #689

Test plan

• 37 new unit tests in InlineDiffHunkViewerTests covering:
  • Hunk navigation (next/previous, wrap-around, single hunk, empty, stale)
  • Click-outside dismissal (inside/outside hunk, pure deletion, multiple hunks)
  • Hunk position info (index/total, edge cases)
  • Toolbar action enum and accessibility IDs
  • Hunk summary text (addition, deletion, modification, context-only)
  • Expanded line range calculation
  • onHunkDismissed callback behavior
  • HunkToolbarView creation and layout
• Existing InlineDiffExpandTests (22 tests) still pass
• SwiftLint clean
• Full project build succeeds
• Visual verification: click diff marker in gutter, verify toolbar appears above hunk with correct summary
• Verify prev/next navigation cycles through hunks
• Verify Restore button reverts the hunk
• Verify click outside hunk dismisses toolbar
• Verify Escape key dismisses toolbar

[github-actions]

✅ Code Coverage: 76.2%

Threshold: 70%

Coverage is above the minimum threshold.

Generated by CI — see job summary for detailed file-level breakdown.

[batonogov]

Code Review — REQUEST CHANGES

Что сделано хорошо

• Чистая архитектура: HunkToolbarAction enum, InlineDiffProvider static methods, HunkToolbarView как отдельный AppKit view
• Dismiss из всех источников (Escape, click outside, data change) правильно централизован через onHunkDismissed callback
• Wrap-around навигация, stale hunk fallback — продуманные edge cases
• 37 тестов покрывают логику навигации, dismiss, summary, line range
• Refactoring dismissExpandedHunk() — устранило дублирование кода Escape + click outside

---

CRITICAL — masksToBounds + shadow конфликт

Pine/HunkToolbarView.swift, строки 60-89:

layer?.masksToBounds = true   // line 61
// ...
layer?.shadowRadius = 3       // line 88
layer?.shadowOpacity = 1      // line 89

masksToBounds = true обрезает shadow. Тень НИКОГДА не будет видна. Это либо мёртвый код (shadow), либо баг (нет тени у toolbar). Нужно убрать masksToBounds = true и вместо этого использовать layer?.cornerCurve = .continuous только для cornerRadius, или сделать два слоя (inner + outer).

---

CRITICAL — динамические цвета не обновляются при смене appearance

Pine/HunkToolbarView.swift, строки 63-82:

layer?.backgroundColor = bgColor.cgColor и layer?.borderColor = borderColor.cgColor — .cgColor вычисляется ОДИН РАЗ при init. При переключении Dark/Light mode цвета останутся от первоначальной темы. Нужен override viewDidChangeEffectiveAppearance() или updateLayer() для пересчёта layer?.backgroundColor и layer?.borderColor.

Аналогичная проблема у separator в makeSeparator() — layer?.backgroundColor не обновится.

---

IMPORTANT — O(n) линейный скан в lineNumberAtPoint на каждый mouseDown

Pine/CodeEditorView.swift, lineNumberAtPoint(_:), строки 540-543:

for i in 0..<min(charIndex, source.length) where source.character(at: i) == ASCII.newline {
    line += 1
}

Для файла в 100K строк это ~100K итераций на каждый клик мыши. В Coordinator уже есть lineStartsCache: LineStartsCache с O(log n) binary search. lineNumberAtPoint должен использовать его, а не сканировать текст заново. Проблема: lineStartsCache живёт в Coordinator, а метод — в GutterTextView. Нужно либо передать cache в GutterTextView, либо вынести логику в Coordinator.

---

IMPORTANT — дублирование line-to-charOffset кода

Pine/CodeEditorView.swift, строки 1952 и 2037:

Один и тот же паттерн "while loop считает newline" повторяется в positionToolbar и scrollToHunk. Плюс третий раз в lineNumberAtPoint. Нужно вынести в private helper method (или использовать lineStartsCache).

---

IMPORTANT — toolbar не зануляется при hideHunkToolbar

Pine/CodeEditorView.swift, hideHunkToolbar():

func hideHunkToolbar() {
    hunkToolbar?.isHidden = true
    hunkToolbar?.removeFromSuperview()
}

Toolbar удаляется из view hierarchy, но ссылка hunkToolbar сохраняется. При следующем showHunkToolbar переиспользуется старый экземпляр (if let existing = hunkToolbar). Это не memory leak, но при повторном показе toolbar будет addSubview ЗАНОВО. Это ок как design choice, но стоит добавить комментарий, что переиспользование — intentional. Либо занулить: hunkToolbar = nil в hideHunkToolbar.

---

IMPORTANT — accessibility IDs не в AccessibilityIdentifiers.swift

Toolbar accessibility IDs ("hunk-toolbar", "hunk-toolbar-summary", "hunk-toolbar-previous" и т.д.) захардкожены в HunkToolbarView и HunkToolbarAction, но не добавлены в Pine/AccessibilityIdentifiers.swift — нарушение проектного соглашения. Все accessibility identifiers должны быть в одном месте.

---

IMPORTANT — tooltip строки не локализованы

"Previous Change", "Next Change", "Restore", "Dismiss" захардкожены прямо в HunkToolbarView.swift. По conventions проекта строки должны идти в Strings.swift и Localizable.xcstrings.

---

SUGGESTION — тест hunkToolbarViewActionCallback не тестирует реальное нажатие кнопки

toolbar.onAction?(.restore)  // это ручной вызов callback, не simulate button click

Лучше бы через button.performClick(nil). Текущий тест проверяет, что Swift closures работают, а не что кнопки подключены к actions.

---

SUGGESTION — нет confirmation для Restore action

Restore необратимо откатывает изменения в hunk. Нет NSAlert/confirmation dialog. VS Code показывает confirmation для revert. Стоит хотя бы создать issue если не хотите делать сейчас.

---

Итого для merge:

1. Fix masksToBounds + shadow conflict
2. Fix dynamic color update on appearance change
3. Использовать lineStartsCache вместо O(n) scan в lineNumberAtPoint
4. Вынести дублированный line-to-charOffset код в helper
5. Перенести accessibility IDs в AccessibilityIdentifiers.swift
6. Локализовать строки в Strings.swift

[batonogov]

Повторное ревью PR #691

Предыдущие замечания — статус

Все 6 замечаний из прошлого ревью исправлены:

1. masksToBounds — убран, shadow видна. Добавлен тест hunkToolbarViewShadowNotClipped.
2. viewDidChangeEffectiveAppearance — переопределён, вызывает updateAppearanceColors(). Фоны и бордеры обновляются.
3. O(log n) lineNumberAtPoint — использует lineStartsCache с binary search, fallback на O(n).
4. Единый charOffsetForLine — один static метод с cache parameter, 1-based нумерация.
5. Accessibility IDs — 6 ID в AccessibilityIdentifiers.swift.
6. Tooltip strings — 4 строки в Strings.swift + Localizable.xcstrings (en/ru).

---

Новые замечания

Important: Separator .cgColor не обновляется при смене темы

HunkToolbarView.makeSeparator() (строка ~164) устанавливает layer?.backgroundColor через .cgColor один раз при создании view:

sep.layer?.backgroundColor = NSColor.separatorColor.withAlphaComponent(0.3).cgColor

Этот .cgColor — снимок текущей темы. При переключении Dark/Light сепараторы останутся в старом цвете. updateAppearanceColors() обновляет фон и бордер основного view, но не трогает сепараторы.

Fix: Сохранить ссылки на separator views и обновлять их layer?.backgroundColor внутри updateAppearanceColors(), аналогично тому, как обновляются borderColor и backgroundColor.

Suggestion: Дублированные MARK-комментарии

В GutterTextView два почти идентичных MARK:

• Строка 515: // MARK: - Click outside hunk dismisses expanded hunk
• Строка 565: // MARK: - Click outside hunk dismisses expanded inline diff

mouseDown под первым, shouldDismissHunkOnClick / dismissExpandedHunk под вторым, а keyDown (Escape) между ними. Логически это одна секция "Hunk dismiss" — стоит объединить в один MARK и сгруппировать mouseDown, shouldDismissHunkOnClick, dismissExpandedHunk, keyDown рядом.

[batonogov]

Code Review -- APPROVED (self-PR, cannot approve via API)

Separator views fix -- VERIFIED

Fix is correct: separatorViews stored as private(set), updated in updateAppearanceColors(), called from viewDidChangeEffectiveAppearance(). Test covers the scenario.

Minor issues (non-blocking)

1. Double hideHunkToolbar() calls in handleHunkToolbarAction: both .dismiss and .restore cases trigger onHunkDismissed callback (via setting expandedHunkID = nil) which calls hideHunkToolbar(), then explicitly call hideHunkToolbar() again. Idempotent so no bug, but redundant.
2. Header comment mismatch in HunkToolbarView.swift: says ← Prev | ↑ summary (2/5) ↓ | Restore | ✕ but actual order is Prev Next | summary | Restore Dismiss.

Positives

• Excellent test coverage (457 lines, edge cases, boundary conditions)
• Clean charOffsetForLine 0-based to 1-based migration
• Proper [weak self] in callbacks
• Correct xcstrings formatting
• Full accessibility identifiers