From 196325d2b256ae61456ebc563204f98f858e2210 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 20 Oct 2026 15:08:35 +0000 Subject: [PATCH] refactor(tool): draw patch hunks without DiffView Hunk cards now use the same ScrollBox as whole-file cards, so the DiffView adapter and its controller are gone. Also drop the unreachable preview-only branches, the disabled failed-chip refresh action, and the unread MCP inspector instructions field. Co-authored-by: Leo Farias --- .../lib/src/model/inspector_controller.dart | 3 - .../lib/src/session/live_mcp_session.dart | 3 - .../lib/src/session/mcp_session.dart | 3 - .../mcp_inspector/test/fake_mcp_session.dart | 3 - .../tool/patch_manager/display_models.dart | 10 +- .../tool/patch_manager/patch_manager_app.dart | 12 +- .../patch_manager/patch_manager_widgets.dart | 191 +++++------------- 7 files changed, 48 insertions(+), 177 deletions(-) diff --git a/packages/noir/tool/mcp_inspector/lib/src/model/inspector_controller.dart b/packages/noir/tool/mcp_inspector/lib/src/model/inspector_controller.dart index 9dbf750a..f9d918e8 100644 --- a/packages/noir/tool/mcp_inspector/lib/src/model/inspector_controller.dart +++ b/packages/noir/tool/mcp_inspector/lib/src/model/inspector_controller.dart @@ -120,9 +120,6 @@ final class InspectorController extends ChangeNotifier { /// The capability names the server advertised. Set get capabilities => session.capabilities; - /// The server's usage instructions, when it supplies them. - String? get instructions => session.instructions; - /// Why the session failed, when it did. String? get errorMessage => _errorMessage; diff --git a/packages/noir/tool/mcp_inspector/lib/src/session/live_mcp_session.dart b/packages/noir/tool/mcp_inspector/lib/src/session/live_mcp_session.dart index fe9d39c3..42d1a8f7 100644 --- a/packages/noir/tool/mcp_inspector/lib/src/session/live_mcp_session.dart +++ b/packages/noir/tool/mcp_inspector/lib/src/session/live_mcp_session.dart @@ -97,9 +97,6 @@ final class LiveMcpSession implements McpSession { @override String? get protocolVersion => _client?.getProtocolVersion(); - @override - String? get instructions => _client?.getInstructions(); - @override Set get capabilities { final advertised = _client?.getServerCapabilities(); diff --git a/packages/noir/tool/mcp_inspector/lib/src/session/mcp_session.dart b/packages/noir/tool/mcp_inspector/lib/src/session/mcp_session.dart index 893e2f45..1f82c00a 100644 --- a/packages/noir/tool/mcp_inspector/lib/src/session/mcp_session.dart +++ b/packages/noir/tool/mcp_inspector/lib/src/session/mcp_session.dart @@ -546,9 +546,6 @@ abstract interface class McpSession { /// The capability names the server advertised, such as `tools`. Set get capabilities; - /// The server's usage instructions, when it supplies them. - String? get instructions; - /// Every JSON-RPC message this session carries. Stream get protocol; diff --git a/packages/noir/tool/mcp_inspector/test/fake_mcp_session.dart b/packages/noir/tool/mcp_inspector/test/fake_mcp_session.dart index dc4f1994..52e33c1f 100644 --- a/packages/noir/tool/mcp_inspector/test/fake_mcp_session.dart +++ b/packages/noir/tool/mcp_inspector/test/fake_mcp_session.dart @@ -74,9 +74,6 @@ final class FakeMcpSession implements McpSession { @override String? get protocolVersion => _connected ? '2026-07-28' : null; - @override - String? get instructions => _connected ? 'Fake instructions.' : null; - @override Set get capabilities => _connected ? const {'tools', 'resources', 'prompts'} : const {}; diff --git a/packages/noir/tool/patch_manager/display_models.dart b/packages/noir/tool/patch_manager/display_models.dart index 8d19b396..59a5954f 100644 --- a/packages/noir/tool/patch_manager/display_models.dart +++ b/packages/noir/tool/patch_manager/display_models.dart @@ -18,7 +18,7 @@ enum PatchLayoutMode { bool get isMedium => this == PatchLayoutMode.medium; } -enum PatchActionKind { skip, stage, refresh, staged } +enum PatchActionKind { skip, stage, staged } class PatchActionItem { const PatchActionItem({ @@ -105,7 +105,7 @@ class PatchActionSet { color: PatchTheme.cyan, ), const PatchActionItem( - kind: PatchActionKind.refresh, + kind: PatchActionKind.staged, label: 'failed', selected: true, enabled: false, @@ -258,10 +258,6 @@ class PatchFileRowModel { additionCount = null; deletionCount = null; fallbackMetadata = compact ? 'binary' : 'binary file'; - } else if (!file.canStageContent && !file.canStageWholeFile) { - additionCount = null; - deletionCount = null; - fallbackMetadata = compact ? 'preview' : 'preview only'; } else { additionCount = file.additionCount; deletionCount = file.deletionCount; @@ -283,8 +279,6 @@ class PatchFileRowModel { /// Short basename of the file, already truncated to fit the current layout. final String basename; - /// Single-letter file-status glyph (M/A/D/R/B/!/U/S/K) — derived from both - /// the diff status and the review status. final String fileStatusLetter; /// One-cell file-kind glyph shown before the basename. diff --git a/packages/noir/tool/patch_manager/patch_manager_app.dart b/packages/noir/tool/patch_manager/patch_manager_app.dart index 4b34f892..261cef45 100644 --- a/packages/noir/tool/patch_manager/patch_manager_app.dart +++ b/packages/noir/tool/patch_manager/patch_manager_app.dart @@ -6,7 +6,6 @@ import 'dart:math' as math; import 'package:meta/meta.dart'; import 'package:noir/noir.dart' hide DiffFile, DiffHunk, DiffLine, UnifiedDiffParser; -import 'package:noir/noir.dart' as noir_diff; import 'package:noir/noir_low_level.dart'; import 'display_models.dart'; import 'git_path.dart'; @@ -171,7 +170,6 @@ class _PatchManagerViewState extends State { final _diffFocus = FocusNode(); late final List _focusRing; late ScrollController _diffScroll; - final _diffViewController = noir_diff.DiffViewController(); String? _diffScrollTarget; int _diffAnchorGeneration = 0; late String _status; @@ -223,7 +221,6 @@ class _PatchManagerViewState extends State { ..dispose(); } _diffScroll.dispose(); - _diffViewController.dispose(); super.dispose(); } @@ -336,12 +333,7 @@ class _PatchManagerViewState extends State { !_controller.hasFiles) { return; } - final file = _controller.currentFile; - if (file.canStageContent) { - _diffViewController.jumpToHunk(_controller.selectedHunkIndex); - // Patch Manager rows are variable-height cards. DiffView establishes - // the semantic hunk anchor; this correction retains the selected - // line/card offset used by the review workflow. + if (_controller.currentFile.canStageContent) { _diffScroll.jumpTo(_selectedSectionScrollOffset().toDouble()); } }), @@ -679,13 +671,11 @@ class _PatchManagerViewState extends State { controller: _controller, focusNode: _diffFocus, scrollController: _diffScroll, - diffViewController: _diffViewController, stagingEnabled: !_refreshRequired, onSkipHunk: _skipHunk, onStageHunk: _stageHunk, onSkipWholeFile: (_) => _skipCurrentReviewTarget(), onStageWholeFile: _stageWholeFile, - onRefresh: _refresh, ), ), ), diff --git a/packages/noir/tool/patch_manager/patch_manager_widgets.dart b/packages/noir/tool/patch_manager/patch_manager_widgets.dart index 32b25049..37651458 100644 --- a/packages/noir/tool/patch_manager/patch_manager_widgets.dart +++ b/packages/noir/tool/patch_manager/patch_manager_widgets.dart @@ -4,7 +4,6 @@ import 'dart:math' as math; import 'package:noir/noir.dart' hide DiffFile, DiffHunk, DiffLine, UnifiedDiffParser; -import 'package:noir/noir.dart' as noir_diff; import 'display_models.dart'; import 'git_path.dart'; import 'models.dart'; @@ -397,43 +396,31 @@ class PatchDiffSurface extends StatelessWidget { required this.controller, required this.focusNode, required this.scrollController, - required this.diffViewController, required this.stagingEnabled, this.onSkipHunk, this.onStageHunk, this.onSkipWholeFile, this.onStageWholeFile, - this.onRefresh, }); final PatchReviewController controller; final FocusNode focusNode; final ScrollController scrollController; - final noir_diff.DiffViewController diffViewController; final bool stagingEnabled; final void Function(DiffHunk hunk)? onSkipHunk; final void Function(DiffHunk hunk)? onStageHunk; final void Function(WholeFileChange change)? onSkipWholeFile; final void Function(WholeFileChange change)? onStageWholeFile; - final VoidCallback? onRefresh; @override Widget build(BuildContext context) { final file = controller.currentFile; - if (!file.canStageContent && !file.canStageWholeFile) { - return const Text( - 'This file has no reviewable changes.', - style: TextStyle(color: PatchTheme.amber), - ); - } - final hunkLabel = file.canStageContent ? 'hunk ${controller.selectedHunkIndex + 1} of ${file.hunks.length}' : WholeFileChangeDisplay.fromChange( file.wholeFileStageUnit!, ).title.toLowerCase(); final incompletePreviewLabel = _incompletePreviewHeaderLabel(file); - final diffAdapter = file.canStageContent ? _PatchDiffAdapter(file) : null; return Column( crossAxisAlignment: CrossAxisAlignment.stretch, children: [ @@ -483,37 +470,56 @@ class PatchDiffSurface extends StatelessWidget { PatchDiffFilterBar(controller: controller, file: file), const SizedBox(height: 1), Expanded( - child: diffAdapter == null - ? ScrollBox( - focusNode: focusNode, - controller: scrollController, - child: Column( - crossAxisAlignment: CrossAxisAlignment.stretch, - children: _wholeFileWidgets(file), - ), - ) - : noir_diff.DiffView( - document: diffAdapter.document, - controller: diffViewController, - scrollController: scrollController, - focusNode: focusNode, - selectable: false, - showLineNumbers: false, - rowBuilder: (_, _, hunk, line, _) => diffAdapter.buildRow( - hunk: hunk, - line: line, - controller: controller, - stagingEnabled: stagingEnabled, - onSkipHunk: onSkipHunk, - onStageHunk: onStageHunk, - onRefresh: onRefresh, - ), - ), + child: ScrollBox( + focusNode: focusNode, + controller: scrollController, + child: Column( + crossAxisAlignment: CrossAxisAlignment.stretch, + children: file.canStageContent + ? _hunkWidgets(file) + : _wholeFileWidgets(file), + ), + ), ), ], ); } + List _hunkWidgets(DiffFile file) { + final widgets = []; + for (var hunkIndex = 0; hunkIndex < file.hunks.length; hunkIndex++) { + final hunk = file.hunks[hunkIndex]; + final selected = hunkIndex == controller.selectedHunkIndex; + widgets + ..add(PatchHunkBorderRow(selected: selected, top: true)) + ..add( + PatchHunkCard( + controller: controller, + hunk: hunk, + selected: selected, + stagingEnabled: stagingEnabled, + onSkipHunk: onSkipHunk, + onStageHunk: onStageHunk, + ), + ); + for (var lineIndex = 0; lineIndex < hunk.lines.length; lineIndex++) { + widgets.add( + PatchDiffLine( + controller: controller, + hunk: hunk, + lineIndex: lineIndex, + framed: true, + selectedHunk: selected, + ), + ); + } + widgets + ..add(PatchHunkBorderRow(selected: selected, top: false)) + ..add(const SizedBox(height: 1)); + } + return widgets; + } + List _wholeFileWidgets(DiffFile file) { final change = file.wholeFileStageUnit!; final widgets = [ @@ -523,7 +529,6 @@ class PatchDiffSurface extends StatelessWidget { stagingEnabled: stagingEnabled, onSkip: onSkipWholeFile, onStage: onStageWholeFile, - onRefresh: onRefresh, ), ]; final notice = _untrackedPreviewNotice(file); @@ -556,106 +561,6 @@ class PatchDiffSurface extends StatelessWidget { } } -final class _PatchDiffAdapter { - _PatchDiffAdapter(DiffFile file) { - final presentationHunks = []; - for (var hunkIndex = 0; hunkIndex < file.hunks.length; hunkIndex++) { - final hunk = file.hunks[hunkIndex]; - final presentationLines = []; - for (var lineIndex = 0; lineIndex < hunk.lines.length; lineIndex++) { - final line = hunk.lines[lineIndex]; - final presentation = noir_diff.DiffLine( - kind: switch (line.type) { - DiffLineType.context => noir_diff.DiffLineKind.context, - DiffLineType.addition => noir_diff.DiffLineKind.addition, - DiffLineType.deletion => noir_diff.DiffLineKind.deletion, - DiffLineType.noNewlineMarker => - noir_diff.DiffLineKind.noNewlineMarker, - }, - text: line.text, - oldLineNumber: line.oldLineNumber, - newLineNumber: line.newLineNumber, - ); - presentationLines.add(presentation); - _lines[presentation] = (hunk: hunk, lineIndex: lineIndex); - } - final presentation = noir_diff.DiffHunk( - oldStart: hunk.oldStart, - oldCount: hunk.oldCount, - newStart: hunk.newStart, - newCount: hunk.newCount, - header: hunk.sectionHeading, - lines: presentationLines, - ); - presentationHunks.add(presentation); - _hunks[presentation] = (hunk: hunk, hunkIndex: hunkIndex); - } - document = noir_diff.DiffDocument([ - noir_diff.DiffFile( - oldPath: file.oldPath == null - ? null - : GitPathPresentation.asciiSafe(file.oldPath!).full, - newPath: file.newPath == null - ? null - : GitPathPresentation.asciiSafe(file.newPath!).full, - hunks: presentationHunks, - ), - ]); - } - - late final noir_diff.DiffDocument document; - final Map _hunks = {}; - final Map _lines = {}; - - Widget buildRow({ - required noir_diff.DiffHunk hunk, - required noir_diff.DiffLine? line, - required PatchReviewController controller, - required bool stagingEnabled, - required void Function(DiffHunk hunk)? onSkipHunk, - required void Function(DiffHunk hunk)? onStageHunk, - required VoidCallback? onRefresh, - }) { - final hunkRecord = _hunks[hunk]!; - final selectedHunk = hunkRecord.hunkIndex == controller.selectedHunkIndex; - if (line == null) { - return Column( - crossAxisAlignment: CrossAxisAlignment.stretch, - children: [ - PatchHunkBorderRow(selected: selectedHunk, top: true), - PatchHunkCard( - controller: controller, - hunk: hunkRecord.hunk, - selected: selectedHunk, - stagingEnabled: stagingEnabled, - onSkipHunk: onSkipHunk, - onStageHunk: onStageHunk, - onRefresh: onRefresh, - ), - ], - ); - } - final lineRecord = _lines[line]!; - final last = lineRecord.lineIndex == lineRecord.hunk.lines.length - 1; - return Column( - crossAxisAlignment: CrossAxisAlignment.stretch, - children: [ - PatchDiffLine( - controller: controller, - hunk: lineRecord.hunk, - lineIndex: lineRecord.lineIndex, - framed: true, - selectedHunk: selectedHunk, - ), - if (last) ...[ - PatchHunkBorderRow(selected: selectedHunk, top: false), - const SizedBox(height: 1), - ], - ], - ); - } -} - String? _incompletePreviewHeaderLabel(DiffFile file) => switch (file.untrackedPreviewState) { UntrackedPreviewState.binary => 'binary preview', @@ -762,7 +667,6 @@ class PatchWholeFileCard extends StatelessWidget { required this.stagingEnabled, this.onSkip, this.onStage, - this.onRefresh, }); final PatchReviewController controller; @@ -770,7 +674,6 @@ class PatchWholeFileCard extends StatelessWidget { final bool stagingEnabled; final void Function(WholeFileChange change)? onSkip; final void Function(WholeFileChange change)? onStage; - final VoidCallback? onRefresh; @override Widget build(BuildContext context) { @@ -820,7 +723,6 @@ class PatchWholeFileCard extends StatelessWidget { onStage == null ? null : () => onStage!(change), PatchActionKind.skip => onSkip == null ? null : () => onSkip!(change), - PatchActionKind.refresh => onRefresh, PatchActionKind.staged => null, }, ), @@ -847,7 +749,6 @@ class PatchHunkCard extends StatelessWidget { required this.stagingEnabled, this.onSkipHunk, this.onStageHunk, - this.onRefresh, }); final PatchReviewController controller; @@ -856,7 +757,6 @@ class PatchHunkCard extends StatelessWidget { final bool stagingEnabled; final void Function(DiffHunk hunk)? onSkipHunk; final void Function(DiffHunk hunk)? onStageHunk; - final VoidCallback? onRefresh; @override Widget build(BuildContext context) { @@ -915,7 +815,6 @@ class PatchHunkCard extends StatelessWidget { PatchActionKind.skip => onSkipHunk == null ? null : () => onSkipHunk!(hunk), PatchActionKind.stage => onStageHunk == null ? null : () => onStageHunk!(hunk), - PatchActionKind.refresh => onRefresh, PatchActionKind.staged => null, }; }