Skip to content

fix: Ignore path conflicts and use spawn instead of exec #116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 21, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 61 additions & 5 deletions src/__tests__/cli-tests.ts
Original file line number Diff line number Diff line change
@@ -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']);
});
});
});

2 changes: 1 addition & 1 deletion src/__tests__/utils-tests.ts
Original file line number Diff line number Diff line change
@@ -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`);
});
29 changes: 23 additions & 6 deletions src/cli.ts
Original file line number Diff line number Diff line change
@@ -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 {
4 changes: 2 additions & 2 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -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 {