Warning dialog for converting changes to suggestions can be incorrect and vague (#6440)

Fixes #6263

Author: alexr00

src/common/diffHunk.ts

@@ -179,26 +179,83 @@ export function parsePatch(patch: string): DiffHunk[] {
 	let diffHunkIter = diffHunkReader.next();
 	const diffHunks: DiffHunk[] = [];
 
-	const right: string[] = [];
 	while (!diffHunkIter.done) {
 		const diffHunk = diffHunkIter.value;
 		diffHunks.push(diffHunk);
+		diffHunkIter = diffHunkReader.next();
+	}
 
-		for (let j = 0; j < diffHunk.diffLines.length; j++) {
-			const diffLine = diffHunk.diffLines[j];
-			if (diffLine.type === DiffChangeType.Delete || diffLine.type === DiffChangeType.Control) {
-			} else if (diffLine.type === DiffChangeType.Add) {
-				right.push(diffLine.text);
-			} else {
-				const codeInFirstLine = diffLine.text;
-				right.push(codeInFirstLine);
+	return diffHunks;
+}
+
+/**
+ * Split a hunk into smaller hunks based on the context lines. Position in hunk and control lines are not preserved.
+ */
+export function splitIntoSmallerHunks(hunk: DiffHunk): DiffHunk[] {
+	const splitHunks: DiffHunk[] = [];
+	const newHunk = (fromLine: DiffLine) => {
+		return {
+			diffLines: [],
+			newLength: 0,
+			oldLength: 0,
+			oldLineNumber: fromLine.oldLineNumber,
+			newLineNumber: fromLine.newLineNumber,
+			positionInHunk: 0
+		};
+	};
+
+	// Split hunk into smaller hunks on context lines.
+	// Context lines will be duplicated across the new smaller hunks
+	let currentHunk: DiffHunk | undefined;
+	let nextHunk: DiffHunk | undefined;
+
+	const addLineToHunk = (hunk: DiffHunk, line: DiffLine) => {
+		hunk.diffLines.push(line);
+		if (line.type === DiffChangeType.Delete) {
+			hunk.oldLength++;
+		} else if (line.type === DiffChangeType.Add) {
+			hunk.newLength++;
+		} else if (line.type === DiffChangeType.Context) {
+			hunk.oldLength++;
+			hunk.newLength++;
+		}
+	};
+	const hunkHasChanges = (hunk: DiffHunk) => {
+		return hunk.diffLines.some(line => line.type !== DiffChangeType.Context);
+	};
+	const hunkHasSandwichedChanges = (hunk: DiffHunk) => {
+		return hunkHasChanges(hunk) && hunk.diffLines[hunk.diffLines.length - 1].type === DiffChangeType.Context;
+	};
+
+	for (const line of hunk.diffLines) {
+		if (line.type === DiffChangeType.Context) {
+			if (!currentHunk) {
+				currentHunk = newHunk(line);
+			}
+			addLineToHunk(currentHunk, line);
+			if (hunkHasSandwichedChanges(currentHunk)) {
+				if (!nextHunk) {
+					nextHunk = newHunk(line);
+				}
+				addLineToHunk(nextHunk, line);
+			}
+		} else if (currentHunk) {
+			if (hunkHasSandwichedChanges(currentHunk)) {
+				splitHunks.push(currentHunk);
+				currentHunk = nextHunk!;
+				nextHunk = undefined;
+			}
+			if ((line.type === DiffChangeType.Delete) || (line.type === DiffChangeType.Add)) {
+				addLineToHunk(currentHunk, line);
 			}
 		}
+	}
 
-		diffHunkIter = diffHunkReader.next();
+	if (currentHunk) {
+		splitHunks.push(currentHunk);
 	}
 
-	return diffHunks;
+	return splitHunks;
 }
 
 export function getModifiedContentFromDiffHunk(originalContent: string, patch: string) {

src/view/reviewManager.ts

@@ -8,7 +8,7 @@ import * as vscode from 'vscode';
 import type { Branch, Repository } from '../api/api';
 import { GitApiImpl, GitErrorCodes, Status } from '../api/api1';
 import { openDescription } from '../commands';
-import { DiffChangeType, DiffHunk, parsePatch } from '../common/diffHunk';
+import { DiffChangeType, DiffHunk, parsePatch, splitIntoSmallerHunks } from '../common/diffHunk';
 import { commands } from '../common/executeCommands';
 import { GitChangeType, InMemFileChange, SlimFileChange } from '../common/file';
 import Logger from '../common/logger';
@@ -723,10 +723,12 @@ export class ReviewManager {
 		let oldLineNumber = hunk.oldLineNumber;
 		let oldLength = hunk.oldLength;
 
-		// start at 1 to skip the control line
-		let i = 1;
+		let i = 0;
 		for (; i < hunk.diffLines.length; i++) {
 			const line = hunk.diffLines[i];
+			if (line.type === DiffChangeType.Control) {
+				continue;
+			}
 			if (line.type === DiffChangeType.Context) {
 				oldLineNumber++;
 				oldLength--;
@@ -749,7 +751,7 @@ export class ReviewManager {
 			// we have only inserted lines, so we need to include a context line so that
 			// there's a line to anchor the suggestion to
 			if (i > 1) {
-				// include from the begginning of the hunk
+				// include from the beginning of the hunk
 				i--;
 				oldLineNumber--;
 				oldLength++;
@@ -789,7 +791,7 @@ export class ReviewManager {
 				if (!resourceStrings.includes(changeFile.uri.toString()) || (changeFile.status !== Status.MODIFIED)) {
 					return;
 				}
-				diff = parsePatch(await this._folderRepoManager.repository.diffWithHEAD(changeFile.uri.fsPath));
+				diff = parsePatch(await this._folderRepoManager.repository.diffWithHEAD(changeFile.uri.fsPath)).map(hunk => splitIntoSmallerHunks(hunk)).flat();
 				await Promise.allSettled(diff.map(async hunk => {
 					try {
 						await this._reviewCommentController?.createSuggestionsFromChanges(changeFile.uri, this.convertDiffHunkToSuggestion(hunk));