From 37dcf7ee011dea9b399a9fc4c0b57f940452fab0 Mon Sep 17 00:00:00 2001 From: Rodrigo Fernandes Date: Sat, 20 Feb 2021 11:10:05 +0000 Subject: [PATCH] fix: Ignore path conflicts and use spawn instead of exec --- src/__tests__/cli-tests.ts | 66 +++++++++++++++++++++++++++++++++--- src/__tests__/utils-tests.ts | 2 +- src/cli.ts | 29 ++++++++++++---- src/utils.ts | 4 +-- 4 files changed, 87 insertions(+), 14 deletions(-) diff --git a/src/__tests__/cli-tests.ts b/src/__tests__/cli-tests.ts index d15855a..4b0c984 100644 --- a/src/__tests__/cli-tests.ts +++ b/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']); + }); }); }); diff --git a/src/__tests__/utils-tests.ts b/src/__tests__/utils-tests.ts index 0fd865a..5ce7a7a 100644 --- a/src/__tests__/utils-tests.ts +++ b/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`); }); diff --git a/src/cli.ts b/src/cli.ts index 28a20cf..0e6585a 100644 --- a/src/cli.ts +++ b/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 { diff --git a/src/utils.ts b/src/utils.ts index 58b3688..2229e91 100644 --- a/src/utils.ts +++ b/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 {