refactor: move diff line counting logic to separate module

src/git/diff_line_count.test.ts

+import { diffFile, mrVersion } from '../test_utils/entities';
+import { getAddedLinesForFile } from './diff_line_count';
+
+describe('diff_line_count', () => {
+  describe('getAddedLinesForFile', () => {
+    const sevenNewLinesHunk = [
+      '@@ -0,0 +1,7 @@',
+      '+new file 2',
+      '+',
+      '+12',
+      '+34',
+      '+56',
+      '+',
+      '+,,,',
+      '',
+    ].join('\n');
+
+    const hunkWithAddedAndRemovedLines = [
+      `@@ -10,17 +10,17 @@`,
+      ` //  This seems to be a result of only some next-line content triggering this issue and other content doesn't. `,
+      ` //  If there is an empty line after, for example, this issue doesn't not occur`,
+      ` `,
+      `-function containingFunction(){`,
+      `-    function subFunction(){`,
+      `+function containingFunction(): void{`,
+      `+    function subFunctionA(): void{`,
+      `         console.log("Some Output");`,
+      `     }`,
+      `     // Issue is not present when the line after the function name and opening { is empty`,
+      `-    function subFunction(){`,
+      `+    function subFunctionB(): void{`,
+      ` `,
+      `         console.log("OPutput");`,
+      `     }`,
+      ` }`,
+      ` `,
+      `-function anotherFunction(){`,
+      `+function anotherFunction(): void{`,
+      `     console.log("Other Output");`,
+      ` }`,
+      ``,
+    ].join('\n');
+
+    const multiHunk = [
+      `@@ -12,9 +12,7 @@`,
+      ` 12`,
+      ` 13`,
+      ` 14`,
+      `-15`,
+      `-16`,
+      `-17`,
+      `+117`,
+      ` 18`,
+      ` 19`,
+      ` 20`,
+      `@@ -38,9 +36,8 @@`,
+      ` 38`,
+      ` 39`,
+      ` 40`,
+      `-41`,
+      `-42`,
+      `-43`,
+      `+441`,
+      `+443`,
+      ` 44`,
+      ` 45`,
+      ` 46`,
+      `@@ -75,7 +72,6 @@`,
+      ` 75`,
+      ` 76`,
+      ` 77`,
+      `-78`,
+      ` 79`,
+      ` 80`,
+      ` 81`,
+      `@@ -98,3 +94,5 @@`,
+      ` 98`,
+      ` 99`,
+      ` 100`,
+      `+101`,
+      `+`,
+    ].join('\n');
+
+    // testing that we can parse diffs of files containing hunk headers
+    const hunkWithHunkHeaders = [
+      `@@ -1,7 +1,4 @@`,
+      ` # Initial readme`,
+      `-8`,
+      `-9`,
+      `-@@ -5,14 +1,20 @@`,
+      ` 0`,
+      ` 1`,
+      ` 2`,
+      `@@ -19,6 +9,7 @@`,
+      ` 7`,
+      ` 8`,
+      ` 9`,
+      `+@@ -5,14 +1,20 @@`,
+      ` @@ -29,3 +11,3 @@`,
+      ` 0`,
+      ` 1`,
+    ].join('\n');
+
+    it.each`
+      hunkName                          | hunk                            | newLines
+      ${'sevenNewLinesHunk'}            | ${sevenNewLinesHunk}            | ${[1, 2, 3, 4, 5, 6, 7]}
+      ${'hunkWithAddedAndRemovedLines'} | ${hunkWithAddedAndRemovedLines} | ${[13, 14, 18, 24]}
+      ${'multiHunk'}                    | ${multiHunk}                    | ${[15, 39, 40, 97, 98]}
+      ${'hunkWithHunkHeaders'}          | ${hunkWithHunkHeaders}          | ${[12]}
+    `('$hunkName gets correctly parsed', ({ hunk, newLines }) => {
+      const testMrVersion = {
+        ...mrVersion,
+        diffs: [{ ...diffFile, diff: hunk }],
+      };
+
+      const ranges = getAddedLinesForFile(testMrVersion, diffFile.new_path);
+
+      expect(ranges).toEqual(newLines);
+    });
+  });
+});

src/git/diff_line_count.ts

+const removeLeadingSlash = (path: string): string => path.replace(/^\//, '');
+
+/**
+ * This method returns line number where in the text document given hunk starts.
+ * Each hunk header contains information about where the hunk starts for old and new version.
+ * `@@ -38,9 +36,8 @@` reads: hunk starts at line 38 of the old version and 36 of the new version.
+ */
+const getHunkStartingLine = (headerString = ''): number | null => {
+  const headerMatch = headerString.match(/@@ -\d+,\d+ \+(\d+),\d+ @@/);
+  return headerMatch && parseInt(headerMatch[1], 10);
+};
+
+const getHunks = (diff: string): string[] => {
+  return diff
+    .replace(/^@@/, '') // remove first @@ because we'll remove all the other @@ by splitting
+    .split('\n@@')
+    .map(h => `@@${h}`); // prepend the removed @@ to all hunks
+};
+
+const getAddedLineNumbers = (hunk: string): number[] => {
+  const hunkLines = hunk.split('\n');
+  const hunkStartingLine = getHunkStartingLine(hunkLines[0]);
+  if (!hunkStartingLine) return [];
+  const noRemovedLines = hunkLines.slice(1, hunkLines.length).filter(l => !l.startsWith('-'));
+  return noRemovedLines.reduce((addedLines: number[], l, i) => {
+    if (l.startsWith('+')) {
+      return [...addedLines, i + hunkStartingLine];
+    }
+    return addedLines;
+  }, []);
+};
+
+export const getAddedLinesForFile = (mrVersion: RestMrVersion, newPath: string): number[] => {
+  // VS Code Uri returns absolute path (leading slash) but GitLab uses relative paths (no leading slash)
+  const diff = mrVersion.diffs.find(d => d.new_path === removeLeadingSlash(newPath));
+  if (!diff) return [];
+  const hunks = getHunks(diff.diff);
+  const changedLinesForHunks = hunks.map(h => getAddedLineNumbers(h));
+
+  return changedLinesForHunks.reduce((acc, changedLines) => [...acc, ...changedLines], []);
+};

src/review/commenting_range_provider.test.ts

@@ -62,100 +62,19 @@ describe('CommentingRangeProvider', () => {
     expect(commentingRangeProvider.provideCommentingRanges(testDocument)).toEqual([]);
   });
 
-  const sevenNewLinesHunk = [
-    '@@ -0,0 +1,7 @@',
-    '+new file 2',
-    '+',
-    '+12',
-    '+34',
-    '+56',
-    '+',
-    '+,,,',
-    '',
-  ].join('\n');
+  const threeNewLinesHunk = ['@@ -0,0 +1,3 @@', '+new file 2', '+', '+12', ''].join('\n');
 
-  const hunkWithAddedAndRemovedLines = [
-    `@@ -10,17 +10,17 @@`,
-    ` //  This seems to be a result of only some next-line content triggering this issue and other content doesn't. `,
-    ` //  If there is an empty line after, for example, this issue doesn't not occur`,
-    ` `,
-    `-function containingFunction(){`,
-    `-    function subFunction(){`,
-    `+function containingFunction(): void{`,
-    `+    function subFunctionA(): void{`,
-    `         console.log("Some Output");`,
-    `     }`,
-    `     // Issue is not present when the line after the function name and opening { is empty`,
-    `-    function subFunction(){`,
-    `+    function subFunctionB(): void{`,
-    ` `,
-    `         console.log("OPutput");`,
-    `     }`,
-    ` }`,
-    ` `,
-    `-function anotherFunction(){`,
-    `+function anotherFunction(): void{`,
-    `     console.log("Other Output");`,
-    ` }`,
-    ``,
-  ].join('\n');
-
-  const multiHunk = [
-    `@@ -12,9 +12,7 @@`,
-    ` 12`,
-    ` 13`,
-    ` 14`,
-    `-15`,
-    `-16`,
-    `-17`,
-    `+117`,
-    ` 18`,
-    ` 19`,
-    ` 20`,
-    `@@ -38,9 +36,8 @@`,
-    ` 38`,
-    ` 39`,
-    ` 40`,
-    `-41`,
-    `-42`,
-    `-43`,
-    `+441`,
-    `+443`,
-    ` 44`,
-    ` 45`,
-    ` 46`,
-    `@@ -75,7 +72,6 @@`,
-    ` 75`,
-    ` 76`,
-    ` 77`,
-    `-78`,
-    ` 79`,
-    ` 80`,
-    ` 81`,
-    `@@ -98,3 +94,5 @@`,
-    ` 98`,
-    ` 99`,
-    ` 100`,
-    `+101`,
-    `+`,
-  ].join('\n');
-
-  it.each`
-    hunkName                          | hunk                            | newLines
-    ${'sevenNewLinesHunk'}            | ${sevenNewLinesHunk}            | ${[1, 2, 3, 4, 5, 6, 7]}
-    ${'hunkWithAddedAndRemovedLines'} | ${hunkWithAddedAndRemovedLines} | ${[13, 14, 18, 24]}
-    ${'multiHunk'}                    | ${multiHunk}                    | ${[15, 39, 40, 97, 98]}
-  `('$hunkName gets correctly parsed', ({ hunk, newLines }) => {
+  it('shows correct commenting ranges for a new file', () => {
     commentingRangeProvider = new CommentingRangeProvider(mr, {
       ...mrVersion,
-      diffs: [{ ...diffFile, diff: hunk }],
+      diffs: [{ ...diffFile, diff: threeNewLinesHunk }],
     });
 
     const ranges = commentingRangeProvider.provideCommentingRanges({
       uri: newFileUri,
     } as vscode.TextDocument);
 
-    // VS Code indexes lines starting with zero, adding 1 to normalize line numbers
-    expect(ranges.map(r => r.start.line + 1)).toEqual(newLines);
+    // VS Code indexes lines starting with zero
+    expect(ranges.map(r => r.start.line)).toEqual([0, 1, 2]);
   });
 });

src/review/commenting_range_provider.ts

 import * as vscode from 'vscode';
 import { FF_COMMENTING_RANGES, REVIEW_URI_SCHEME } from '../constants';
+import { getAddedLinesForFile } from '../git/diff_line_count';
 import { getExtensionConfiguration } from '../utils/get_extension_configuration';
 import { fromReviewUri } from './review_uri';
 
-const removeLeadingSlash = (path: string): string => path.replace(/^\//, '');
-
-/**
- * This method returns line number where in the text document given hunk starts.
- * Each hunk header contains information about where the hunk starts for old and new version.
- * `@@ -38,9 +36,8 @@` reads: hunk starts at line 38 of the old version and 36 of the new version.
- */
-const getHunkStartingLine = (headerString = ''): number | null => {
-  const headerMatch = headerString.match(/@@ -\d+,\d+ \+(\d+),\d+ @@/);
-  return headerMatch && parseInt(headerMatch[1], 10);
-};
-
-const getHunks = (diff: string): string[] => {
-  return diff
-    .replace(/^@@/, '') // remove first @@ because we'll remove all the other @@ by splitting
-    .split('\n@@')
-    .map(h => `@@${h}`); // prepend the removed @@ to all hunks
-};
-
-const getAddedLineNumbers = (hunk: string): number[] => {
-  const hunkLines = hunk.split('\n');
-  const hunkStartingLine = getHunkStartingLine(hunkLines[0]);
-  if (!hunkStartingLine) return [];
-  const noRemovedLines = hunkLines.slice(1, hunkLines.length).filter(l => !l.startsWith('-'));
-  return noRemovedLines.reduce((addedLines: number[], l, i) => {
-    if (l.startsWith('+')) {
-      return [...addedLines, i + hunkStartingLine];
-    }
-    return addedLines;
-  }, []);
-};
-
-export const getAddedLinesFromDiff = (diff: string): number[] => {
-  const hunks = getHunks(diff);
-  const changedLinesForHunks = hunks.map(h => getAddedLineNumbers(h));
-
-  return changedLinesForHunks.reduce((acc, changedLines) => [...acc, ...changedLines], []);
-};
-
 export class CommentingRangeProvider implements vscode.CommentingRangeProvider {
   private mr: RestIssuable;
 
@@ -66,16 +28,9 @@ export class CommentingRangeProvider implements vscode.CommentingRangeProvider {
         new vscode.Range(new vscode.Position(0, 0), new vscode.Position(document.lineCount - 1, 0)),
       ];
     }
-    const fileDiff = this.getNewFileDiff(params.path);
-    if (!fileDiff) return [];
-    const result = getAddedLinesFromDiff(fileDiff.diff);
+    const result = getAddedLinesForFile(this.mrVersion, params.path);
     return result.map(
       l => new vscode.Range(new vscode.Position(l - 1, 0), new vscode.Position(l - 1, 0)),
     );
   }
-
-  private getNewFileDiff(path: string): RestDiffFile | undefined {
-    // VS Code Uri returns absolute path (leading slash) but GitLab uses relative paths (no leading slash)
-    return this.mrVersion.diffs.find(d => d.new_path === removeLeadingSlash(path));
-  }
 }