diff --git a/src/git/diff_line_count.test.ts b/src/git/diff_line_count.test.ts index 59239e1d8dd614afefdfc438155bdd2d1ced3374..66bf7a2733fde1a6d5795f18eabdf76bc03ab233 100644 --- a/src/git/diff_line_count.test.ts +++ b/src/git/diff_line_count.test.ts @@ -100,6 +100,17 @@ describe('diff_line_count', () => { ` 18`, ].join('\n'); + const hunkWithoutNewLines = [ + '@@ -1 +1,4 @@', + '-# Initial readme', + '\\ No newline at end of file', + '+1', + '+2', + '+3', + '+4', + '\\ No newline at end of file', + ].join('\n'); + const testVersion = (diff: string): RestMrVersion => ({ ...mrVersion, diffs: [{ ...diffFile, diff }], @@ -112,6 +123,8 @@ describe('diff_line_count', () => { ${'hunkWithAddedAndRemovedLines'} | ${hunkWithAddedAndRemovedLines} | ${[13, 14, 18, 24]} ${'multiHunk'} | ${multiHunk} | ${[15, 39, 40, 97, 98]} ${'hunkWithHunkHeaders'} | ${hunkWithHunkHeaders} | ${[15]} + ${'hunkWithHunkHeaders'} | ${hunkWithHunkHeaders} | ${[15]} + ${'hunkWithoutNewLines'} | ${hunkWithoutNewLines} | ${[1, 2, 3, 4]} `('$hunkName gets correctly parsed', ({ hunk, newLines }) => { const ranges = getAddedLinesForFile(testVersion(hunk), diffFile.new_path); diff --git a/src/git/diff_line_count.ts b/src/git/diff_line_count.ts index 18c7c552bd2031356c72f0128606b6ba65e0e371..72f35c9636285b6267887b03223d2ad0d6484dc3 100644 --- a/src/git/diff_line_count.ts +++ b/src/git/diff_line_count.ts @@ -16,7 +16,7 @@ const isEmpty = (obj: any): boolean => * `@@ -38,9 +36,8 @@` reads: hunk starts at line 38 of the old version and 36 of the new version. */ const getHunkStartingLine = (headerString = ''): { oldStart: number; newStart: number } | null => { - const headerMatch = headerString.match(/@@ -(\d+),\d+ \+(\d+),\d+ @@/); + const headerMatch = headerString.match(/@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/); return ( headerMatch && { oldStart: parseInt(headerMatch[1], 10), @@ -48,6 +48,7 @@ const parseHunk = (hunk: string): HunkLine[] => { assert(header); const result = remainingLines .filter(l => l) // no empty lines + .filter(l => !l.startsWith('\\')) // ignore '\ No newline at end of file' .reduce( ({ oldIndex, newIndex, lines }, line) => { const prefix = line[0]; diff --git a/src/review/commenting_range_provider.test.ts b/src/review/commenting_range_provider.test.ts index b363eadb0a40d407c6825ff46b6250b0deec9633..92f512f39bc9e6938e005949fdc4eda18b7e1b23 100644 --- a/src/review/commenting_range_provider.test.ts +++ b/src/review/commenting_range_provider.test.ts @@ -35,15 +35,31 @@ describe('CommentingRangeProvider', () => { }); it('returns full range (all lines in the document) for old file', () => { - const testDocument = { + const testDocument = ({ uri: oldFileUrl, lineCount: 200, - } as vscode.TextDocument; + lineAt: () => ({ + isEmptyOrWhitespace: false, + }), + } as unknown) as vscode.TextDocument; expect(commentingRangeProvider.provideCommentingRanges(testDocument)).toEqual([ new vscode.Range(new vscode.Position(0, 0), new vscode.Position(199, 0)), ]); }); + it('returns range without the last line for old file if the last line is empty', () => { + const testDocument = ({ + uri: oldFileUrl, + lineCount: 200, + lineAt: () => ({ + isEmptyOrWhitespace: true, + }), + } as unknown) as vscode.TextDocument; + expect(commentingRangeProvider.provideCommentingRanges(testDocument)).toEqual([ + new vscode.Range(new vscode.Position(0, 0), new vscode.Position(198, 0)), + ]); + }); + const threeNewLinesHunk = ['@@ -0,0 +1,3 @@', '+new file 2', '+', '+12', ''].join('\n'); it('shows correct commenting ranges for a new file', () => { diff --git a/src/review/commenting_range_provider.ts b/src/review/commenting_range_provider.ts index addc24b621922252d5de5b2e3f27019240d6f6f9..5ecc9a11aa432c87c35ffa40ae457ceff8cc760a 100644 --- a/src/review/commenting_range_provider.ts +++ b/src/review/commenting_range_provider.ts @@ -3,6 +3,10 @@ import { REVIEW_URI_SCHEME } from '../constants'; import { getAddedLinesForFile } from '../git/diff_line_count'; import { fromReviewUri } from './review_uri'; +const lastLineEmpty = (document: vscode.TextDocument): boolean => { + const lastLIne = document.lineAt(document.lineCount - 1); + return lastLIne.isEmptyOrWhitespace; +}; export class CommentingRangeProvider implements vscode.CommentingRangeProvider { private mr: RestIssuable; @@ -22,9 +26,8 @@ export class CommentingRangeProvider implements vscode.CommentingRangeProvider { } const oldFile = params.commit === this.mrVersion.base_commit_sha; if (oldFile) { - return [ - new vscode.Range(new vscode.Position(0, 0), new vscode.Position(document.lineCount - 1, 0)), - ]; + const endOfRange = lastLineEmpty(document) ? document.lineCount - 2 : document.lineCount - 1; + return [new vscode.Range(new vscode.Position(0, 0), new vscode.Position(endOfRange, 0))]; } const result = getAddedLinesForFile(this.mrVersion, params.path); return result.map(