Skip to content

Commit 7def832

Browse files
geevensinghCopilot
andcommitted
DiffViewer: replace revert-hunk preview with a one-line summary
The preview block was rendering as plain unstyled text in a generic white WPF dialog (Segoe UI, no syntax colors), so even after filtering to changed lines with +/- markers, the result looked like it came from a different app than DiffViewer. Plumbing the editor color scheme, font family, and font size into a confirmation dialog isn't worth the view-layer surface area when the user just needs to confirm intent. Drop the line excerpt entirely and replace it with a one-line count summary the user can read at a glance: Discard this hunk from the working tree? This cannot be undone via git. 1 addition, 1 removal at line 47 in MyMethod(). The reported line number is the first actually-changed line in the hunk (first non-context line's NewLineNumber, falling back to OldLineNumber for pure deletions). hunk.NewStartLine would have pointed at the leading context line DiffService prepends to every hunk, putting the reported position a line or two above where the change actually sits. Function context is the unified-diff '@@ ... @@' trailer, included only when DiffService captured one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 01be2e1 commit 7def832

2 files changed

Lines changed: 49 additions & 34 deletions

File tree

DiffViewer.Tests/ViewModels/MainViewModelContextMenuTests.cs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -337,13 +337,15 @@ await RunOnUiSyncContextAsync(async vm =>
337337
}
338338

339339
[Fact]
340-
public async Task RevertHunkAtCaret_PreviewShowsOnlyChangedLinesWithMarkers()
340+
public async Task RevertHunkAtCaret_PromptDescribesHunkRatherThanPreviewingLines()
341341
{
342-
// Repro of the "useless preview" bug: leading context lines were
343-
// taking up the entire 3-line preview window, so the user saw three
344-
// unchanged lines and zero indication of what was about to be
345-
// discarded. The fix filters out Context lines and prefixes diff
346-
// markers so the preview actually previews the revert.
342+
// The dialog used to show a 3-line preview from the top of the hunk
343+
// - which, because DiffService prepends context lines around every
344+
// hunk, almost always rendered three unchanged lines and zero
345+
// indication of what was about to be discarded. The replacement is
346+
// a one-line summary of additions/removals + line number so the
347+
// user knows what they're confirming without dragging editor
348+
// settings into a plain System.Windows MessageBox-style dialog.
347349
var repo = new FakeRepositoryService(_repoRoot)
348350
{
349351
LeftText = "alpha\nbeta\n",
@@ -366,10 +368,14 @@ await RunOnUiSyncContextAsync(async vm =>
366368
}, repo, git);
367369

368370
seen.Should().NotBeNull();
369-
seen!.Message.Should().Contain("- beta", "deleted line should appear in preview with '-' marker");
370-
seen.Message.Should().Contain("+ gamma", "inserted line should appear in preview with '+' marker");
371-
seen.Message.Should().NotContain("Preview:\nalpha",
372-
"leading context line should be filtered out of preview");
371+
seen!.Message.Should().Contain("1 addition", "the inserted 'gamma' line should be counted");
372+
seen.Message.Should().Contain("1 removal", "the deleted 'beta' line should be counted");
373+
seen.Message.Should().Contain("at line 2",
374+
"the prompt should pin the change to its line number in the working tree");
375+
seen.Message.Should().NotContain("Preview:",
376+
"the old line-by-line preview has been removed");
377+
seen.Message.Should().NotContain("alpha",
378+
"context lines should not leak into the summary");
373379
}
374380

375381
[Fact]

DiffViewer/ViewModels/MainViewModel.cs

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -213,33 +213,10 @@ private async Task RevertHunkAtCaret(HunkActionContext? ctx)
213213
var suppress = _settingsService?.Current.SuppressRevertHunkConfirmation ?? false;
214214
if (!suppress)
215215
{
216-
// Preview the lines that will actually be reverted: filter to
217-
// inserted/deleted/modified (skip context, which is unchanged
218-
// and tells the user nothing about what's being discarded) and
219-
// prefix each with the standard diff marker so + / - reads
220-
// naturally. The earlier "first 3 lines of the hunk" approach
221-
// surfaced only the leading context lines that DiffService adds
222-
// around every hunk, so the dialog showed three unchanged
223-
// lines and no actual changes - useless for confirming intent.
224-
var changedLines = inputs.Hunk.Lines
225-
.Where(l => l.Kind is DiffLineKind.Inserted
226-
or DiffLineKind.Deleted
227-
or DiffLineKind.Modified)
228-
.ToList();
229-
var previewLines = changedLines
230-
.Take(3)
231-
.Select(l => (l.Kind switch
232-
{
233-
DiffLineKind.Inserted => "+ ",
234-
DiffLineKind.Deleted => "- ",
235-
DiffLineKind.Modified => "~ ",
236-
_ => " ",
237-
}) + l.Text);
238216
var resp = ConfirmHandler?.Invoke(new ConfirmationRequest(
239217
Title: "Revert hunk?",
240218
Message: "Discard this hunk from the working tree? This cannot be undone via git.\n\n"
241-
+ "Preview:\n" + string.Join("\n", previewLines)
242-
+ (changedLines.Count > 3 ? "\n…" : ""),
219+
+ DescribeHunk(inputs.Hunk),
243220
ConfirmText: "Revert",
244221
CancelText: "Cancel",
245222
ShowDontAskAgain: true));
@@ -264,6 +241,38 @@ private async Task OpenInExternalEditorAtLine(LineActionContext? ctx)
264241
if (!r.Success) ToastHandler?.Invoke(r.ErrorMessage ?? "Editor launch failed.");
265242
}
266243

244+
// One-line human summary of a hunk for the "Revert hunk?" prompt.
245+
// DiffService never emits DiffLineKind.Modified today (every change is
246+
// expressed as a Deleted+Inserted pair), so additions/removals fully
247+
// describe what the revert will undo. The reported line number is the
248+
// first actually-changed line - not hunk.NewStartLine, which includes
249+
// DiffService's leading context lines and would point a line or two
250+
// above where the user's change really sits. Function context comes
251+
// from the unified-diff "@@ ... @@" trailer when available.
252+
private static string DescribeHunk(DiffHunk hunk)
253+
{
254+
var added = hunk.Lines.Count(l => l.Kind == DiffLineKind.Inserted);
255+
var removed = hunk.Lines.Count(l => l.Kind == DiffLineKind.Deleted);
256+
257+
static string Plural(int n, string singular, string plural)
258+
=> $"{n} {(n == 1 ? singular : plural)}";
259+
260+
var parts = new List<string>(2);
261+
if (added > 0) parts.Add(Plural(added, "addition", "additions"));
262+
if (removed > 0) parts.Add(Plural(removed, "removal", "removals"));
263+
var counts = parts.Count > 0 ? string.Join(", ", parts) : "no line changes";
264+
265+
var firstChange = hunk.Lines.FirstOrDefault(l => l.Kind != DiffLineKind.Context);
266+
var lineNum = firstChange?.NewLineNumber
267+
?? firstChange?.OldLineNumber
268+
?? hunk.NewStartLine;
269+
var location = $"at line {lineNum}";
270+
if (!string.IsNullOrWhiteSpace(hunk.FunctionContext))
271+
location += $" in {hunk.FunctionContext}";
272+
273+
return $"{counts} {location}.";
274+
}
275+
267276
// ---------------- Keyboard-shortcut commands ----------------
268277
// Cross-file / cross-section hunk navigation (F7/F8 family).
269278
// Toggle aliases for toolbar state (Ctrl+W, Ctrl+I, etc.).

0 commit comments

Comments
 (0)