fix: Ignore path conflicts and use spawn instead of exec

Author: rtfpessoa

src/__tests__/cli-tests.ts

@@ -24,13 +24,69 @@ describe('cli', () => {
       expect(readStdinSpy).toHaveBeenCalledWith();
     });
 
-    test('should readStdin when inputType is `command`', async () => {
-      const executeSpy = jest.spyOn(utils, 'execute').mockImplementationOnce(() => '');
+    describe('should execute command when inputType is `command`', () => {
+      test('should pass args to git command and add `--no-color` flag', async () => {
+        const executeSpy = jest.spyOn(utils, 'execute').mockImplementationOnce(() => '');
 
-      await cli.getInput('command', ['lol', 'foo'], []);
+        await cli.getInput('command', ['foo'], []);
 
-      expect(executeSpy).toHaveBeenCalledTimes(1);
-      expect(executeSpy).toHaveBeenCalledWith('git diff "lol" "foo" --no-color ');
+        expect(executeSpy).toHaveBeenCalledTimes(1);
+        expect(executeSpy).toHaveBeenCalledWith('git', ['diff', '--no-color', 'foo']);
+      });
+
+      test('should not add `--no-color` flag if already in args', async () => {
+        const executeSpy = jest.spyOn(utils, 'execute').mockImplementationOnce(() => '');
+
+        await cli.getInput('command', ['--no-color'], []);
+
+        expect(executeSpy).toHaveBeenCalledTimes(1);
+        expect(executeSpy).toHaveBeenCalledWith('git', ['diff', '--no-color']);
+      });
+
+      test('should add default flags if no args are provided', async () => {
+        const executeSpy = jest.spyOn(utils, 'execute').mockImplementationOnce(() => '');
+
+        await cli.getInput('command', [], []);
+
+        expect(executeSpy).toHaveBeenCalledTimes(1);
+        expect(executeSpy).toHaveBeenCalledWith('git', ['diff', '--no-color', '-M', '-C', 'HEAD']);
+      });
+
+      describe('when receiving paths to ignore', () => {
+        test('should add the `--` separator if it is not already in args', async () => {
+          const executeSpy = jest.spyOn(utils, 'execute').mockImplementationOnce(() => '');
+
+          await cli.getInput('command', ['foo'], ['some/path']);
+
+          expect(executeSpy).toHaveBeenCalledTimes(1);
+          expect(executeSpy).toHaveBeenCalledWith('git', ['diff', '--no-color', 'foo', '--', ':(exclude)some/path']);
+        });
+
+        test('should not add `--` flag if it is already in args', async () => {
+          const executeSpy = jest.spyOn(utils, 'execute').mockImplementationOnce(() => '');
+
+          await cli.getInput('command', ['foo', '--', 'other/path'], ['some/path']);
+
+          expect(executeSpy).toHaveBeenCalledTimes(1);
+          expect(executeSpy).toHaveBeenCalledWith('git', [
+            'diff',
+            '--no-color',
+            'foo',
+            '--',
+            'other/path',
+            ':(exclude)some/path',
+          ]);
+        });
+      });
+
+      test('should not add `--` flag when there are no paths to ignore', async () => {
+        const executeSpy = jest.spyOn(utils, 'execute').mockImplementationOnce(() => '');
+
+        await cli.getInput('command', ['bar'], []);
+
+        expect(executeSpy).toHaveBeenCalledTimes(1);
+        expect(executeSpy).toHaveBeenCalledWith('git', ['diff', '--no-color', 'bar']);
+      });
     });
   });
 

src/__tests__/utils-tests.ts

@@ -13,7 +13,7 @@ describe('utils', () => {
 
   test('should execute command in shell', () => {
     const echoedValue = 'echoed string';
-    const result = utils.execute(`echo "${echoedValue}"`);
+    const result = utils.execute('echo', [echoedValue]);
 
     expect(result).toBe(`${echoedValue}\n`);
   });

src/cli.ts

@@ -11,14 +11,31 @@ import * as log from './logger';
 import { Configuration, InputType, DiffyType } from './types';
 import * as utils from './utils';
 
-function runGitDiff(gitArgsArr: string[], ignore: string[]): string {
-  const baseArgs = gitArgsArr.length > 0 ? gitArgsArr.map(arg => `"${arg}"`) : ['-M', '-C', 'HEAD'];
-  const colorArgs = gitArgsArr.indexOf('--no-color') < 0 ? ['--no-color'] : [];
-  const ignoreArgs = ignore.map(file => `":(exclude)${file}"`);
+const defaultArgs = ['-M', '-C', 'HEAD'];
+
+function generateGitDiffArgs(gitArgsArr: string[], ignore: string[]): string[] {
+  const gitDiffArgs: string[] = ['diff'];
+
+  if (!gitArgsArr.includes('--no-color')) gitDiffArgs.push('--no-color');
 
-  const diffCommand = `git diff ${baseArgs.join(' ')} ${colorArgs.join(' ')} ${ignoreArgs.join(' ')}`;
+  if (gitArgsArr.length === 0) Array.prototype.push.apply(gitDiffArgs, defaultArgs);
 
-  return utils.execute(diffCommand);
+  Array.prototype.push.apply(gitDiffArgs, gitArgsArr);
+
+  if (ignore.length > 0) {
+    if (!gitArgsArr.includes('--')) gitDiffArgs.push('--');
+    Array.prototype.push.apply(
+      gitDiffArgs,
+      ignore.map(path => `:(exclude)${path}`),
+    );
+  }
+
+  return gitDiffArgs;
+}
+
+function runGitDiff(gitArgsArr: string[], ignore: string[]): string {
+  const gitDiffArgs = generateGitDiffArgs(gitArgsArr, ignore);
+  return utils.execute('git', gitDiffArgs);
 }
 
 function prepareHTML(diffHTMLContent: string, config: Configuration): string {

src/utils.ts

@@ -28,8 +28,8 @@ export function writeFile(filePath: string, content: string): void {
   return fs.writeFileSync(filePath, content);
 }
 
-export function execute(cmd: string): string {
-  return childProcess.execSync(cmd).toString('utf8');
+export function execute(executable: string, args: string[]): string {
+  return childProcess.spawnSync(executable, args, { encoding: 'utf8' }).stdout;
 }
 
 export function replaceExactly(value: string, searchValue: string, replaceValue: string): string {