Skip to content

DiffViewer: split MainViewModel and DiffPaneViewModel into focused coordinators #29

@geevensingh

Description

@geevensingh

Problem

DiffViewer/ViewModels/MainViewModel.cs is 621 lines and DiffViewer/ViewModels/DiffPaneViewModel.cs is 690 lines. Both have grown to do too many unrelated things, which makes them harder to navigate, test, and refactor safely.

MainViewModel (621 lines) currently owns:

Concern Lines Notes
File-list context-menu commands 88-172 ShowInExplorer, OpenWithDefaultApp, OpenInExternalEditor, StageFile, DeleteFile, AddToGitignore, Copy* (5 variants), RefreshList
Diff-pane context-menu commands 175-227 StageHunkAtCaret, UnstageHunkAtCaret, RevertHunkAtCaret, OpenInExternalEditorAtLine
F7/F8 cross-file navigation 282-398 NavigateNextChange, NavigatePreviousChange, NavigateNextFile, NavigatePreviousFile, NavigateNextSection, NavigatePreviousSection, StepFile, StepSection, FindNextWithChanges, IndexOfSectionContaining
Toolbar toggle aliases 260-274 ToggleIgnoreWhitespace, ToggleVisibleWhitespace, etc.
Clipboard helper 421-439 CopyToClipboard with optional override
Repository-watcher coordination + write-op suspend tokens 30-39, 471-481, 600-684 _suspendTokens, _writeSnapshots, OnBeforeWriteOperation, OnAfterWriteOperation, RepoFileStatsSnapshot
Live-updates pause-handling 42-47, 540+ _missedChangeKindWhilePaused + OnDiffPanePropertyChanged
Settings hook for the View 65-69 SettingsService exposure

DiffPaneViewModel (690 lines) currently owns:

Concern Lines Notes
Settings ↔ observable-property bridging 137-217, 250-279, 530-561 Constructor seed, OnSettingsChanged, OnFontSizeChanged, Zoom*, PersistToolbarToSettings, OnXxxChanged (×5) — repetitive _suppressSettingsWrite plumbing
Color scheme management 175-217 CurrentColorScheme, ColorSchemeChanged, OnSettingsChanged color path
Diff load orchestration 295-360 LoadAsync
Auto-scroll variant 362-381 LoadAndScrollToFirstHunkAsync (just shipped)
Hunk navigation 599-669 TryNavigateNextHunkInFile, TryNavigatePreviousHunkInFile, IsAtLastHunk, IsAtFirstHunk, JumpToFirstHunk, JumpToLastHunk, JumpToHunk, RaiseHunkNav
Hunk patch building (in HunkActionContext path) HunkAtLine, BuildHunkPatchInputs, CurrentEntry
Viewport projection / scroll-by-fraction 127-135, 671-690+ Viewport, RequestScrollByFraction
Inline-document model 39, 45-56, 100, 503-504 InlineDocument, InlineLineHighlights, InlineLineToSourceLines
Placeholder logic 86-94, 312-321, 442-456 PlaceholderMessage, ResolvePlaceholderForShape, ResolveLargeFilePlaceholder

Why it matters

  • New contributors have to load 1300+ lines of cross-cutting state to make any change.
  • Tests are forced into one mega-fixture (MainViewModelKeyboardShortcutTests, MainViewModelContextMenuTests, DiffPaneViewModelTests).
  • The auto-scroll race we just fixed (PR c4ed6f0) was hard to reason about because so much else lives in the same VM.

Suggested split

From MainViewModel:

  1. HunkNavigationCoordinator — owns NavigateNext/PreviousChange/File/Section, StepFile, StepSection, FindNextWithChanges, IndexOfSectionContaining. Takes FileListViewModel, DiffPaneViewModel, and emits commands.
  2. FileContextMenuCoordinator — owns ShowInExplorer, OpenWith*, StageFile, DeleteFile, AddToGitignore, Copy*, RefreshList. Takes IExternalAppLauncher, IGitWriteService, ISettingsService, ToastHandler.
  3. HunkActionCoordinator — owns StageHunkAtCaret, UnstageHunkAtCaret, RevertHunkAtCaret, OpenInExternalEditorAtLine, plus TryGetHunkAction.
  4. WriteOpWatcherSuspender — owns _suspendTokens, _writeSnapshots, OnBeforeWriteOperation, OnAfterWriteOperation, RepoFileStatsSnapshot. Pure plumbing; testable in isolation.

From DiffPaneViewModel:

  1. ToolbarSettingsBridge — owns the seed-from-settings-on-construct dance, the _suppressSettingsWrite flag, the partial OnXxxChanged handlers, PersistToolbarToSettings, and the editor-appearance bridges (FontSize/FontFamily/TabWidth/etc.). About 80 lines lifted out, replaced with a single field ToolbarSettingsBridge _settings.

These are all mechanical extractions — the existing tests should continue to pass with no behaviour change.

Citations

  • DiffViewer/ViewModels/MainViewModel.cs (621 lines)
  • DiffViewer/ViewModels/DiffPaneViewModel.cs (690 lines)
  • DiffViewer.Tests/ViewModels/MainViewModelKeyboardShortcutTests.cs, MainViewModelContextMenuTests.cs, DiffPaneViewModelTests.cs — the test files would also split naturally along these lines.

Severity

Low (no functional impact). Quality-of-life / maintainability win that compounds as more features land.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions