diff --git a/simple-git/src/lib/args/log-format.ts b/simple-git/src/lib/args/log-format.ts index bc8eca10..e401814c 100644 --- a/simple-git/src/lib/args/log-format.ts +++ b/simple-git/src/lib/args/log-format.ts @@ -1,4 +1,3 @@ - export enum LogFormat { NONE = '', STAT = '--stat', diff --git a/simple-git/src/lib/errors/git-error.ts b/simple-git/src/lib/errors/git-error.ts index 3ec12683..cb6231d8 100644 --- a/simple-git/src/lib/errors/git-error.ts +++ b/simple-git/src/lib/errors/git-error.ts @@ -1,4 +1,4 @@ -import { SimpleGitTask } from '../types'; +import type { SimpleGitTask } from '../types'; /** * The `GitError` is thrown when the underlying `git` process throws a diff --git a/simple-git/src/lib/parsers/parse-branch-delete.ts b/simple-git/src/lib/parsers/parse-branch-delete.ts index 15d6d709..86242bbe 100644 --- a/simple-git/src/lib/parsers/parse-branch-delete.ts +++ b/simple-git/src/lib/parsers/parse-branch-delete.ts @@ -23,7 +23,7 @@ const parsers: LineParser[] = [ ]; export const parseBranchDeletions: TaskParser = (stdOut, stdErr) => { - return parseStringResponse(new BranchDeletionBatch(), parsers, stdOut, stdErr); + return parseStringResponse(new BranchDeletionBatch(), parsers, [stdOut, stdErr]); } export function hasBranchDeletionError(data: string, processExitCode: ExitCodes): boolean { diff --git a/simple-git/src/lib/parsers/parse-diff-summary.ts b/simple-git/src/lib/parsers/parse-diff-summary.ts index 2672c5e4..636bcbff 100644 --- a/simple-git/src/lib/parsers/parse-diff-summary.ts +++ b/simple-git/src/lib/parsers/parse-diff-summary.ts @@ -97,5 +97,5 @@ const diffSummaryParsers: Record[]> = { export function getDiffParser(format = LogFormat.NONE) { const parser = diffSummaryParsers[format]; - return (stdOut: string) => parseStringResponse(new DiffSummary(), parser, stdOut); + return (stdOut: string) => parseStringResponse(new DiffSummary(), parser, stdOut, false); } diff --git a/simple-git/src/lib/parsers/parse-fetch.ts b/simple-git/src/lib/parsers/parse-fetch.ts index 3b955a18..1efc9b21 100644 --- a/simple-git/src/lib/parsers/parse-fetch.ts +++ b/simple-git/src/lib/parsers/parse-fetch.ts @@ -26,5 +26,5 @@ export function parseFetchResult (stdOut: string, stdErr: string): FetchResult { branches: [], tags: [], }; - return parseStringResponse(result, parsers, stdOut, stdErr); + return parseStringResponse(result, parsers, [stdOut, stdErr]); } diff --git a/simple-git/src/lib/parsers/parse-pull.ts b/simple-git/src/lib/parsers/parse-pull.ts index 9076fdb2..6888c72d 100644 --- a/simple-git/src/lib/parsers/parse-pull.ts +++ b/simple-git/src/lib/parsers/parse-pull.ts @@ -47,7 +47,7 @@ const errorParsers: LineParser[] = [ ]; export const parsePullDetail: TaskParser = (stdOut, stdErr) => { - return parseStringResponse(new PullSummary(), parsers, stdOut, stdErr); + return parseStringResponse(new PullSummary(), parsers, [stdOut, stdErr]); } export const parsePullResult: TaskParser = (stdOut, stdErr) => { @@ -59,7 +59,7 @@ export const parsePullResult: TaskParser = (stdOut, stdErr) } export function parsePullErrorResult(stdOut: string, stdErr: string) { - const pullError = parseStringResponse(new PullFailedSummary(), errorParsers, stdOut, stdErr); + const pullError = parseStringResponse(new PullFailedSummary(), errorParsers, [stdOut, stdErr]); return pullError.message && pullError; } diff --git a/simple-git/src/lib/parsers/parse-push.ts b/simple-git/src/lib/parsers/parse-push.ts index 9871afa7..ae25a58a 100644 --- a/simple-git/src/lib/parsers/parse-push.ts +++ b/simple-git/src/lib/parsers/parse-push.ts @@ -65,5 +65,5 @@ export const parsePushResult: TaskParser = (stdOut, stdErr) } export const parsePushDetail: TaskParser = (stdOut, stdErr) => { - return parseStringResponse({pushed: []}, parsers, stdOut, stdErr); + return parseStringResponse({pushed: []}, parsers, [stdOut, stdErr]); } diff --git a/simple-git/src/lib/tasks/diff.ts b/simple-git/src/lib/tasks/diff.ts index 3b547baf..5e741ce3 100644 --- a/simple-git/src/lib/tasks/diff.ts +++ b/simple-git/src/lib/tasks/diff.ts @@ -4,7 +4,7 @@ import { isLogFormat, LogFormat, logFormatFromCommand } from '../args/log-format import { getDiffParser } from '../parsers/parse-diff-summary'; import { configurationErrorTask, EmptyTask } from './task'; -export function diffSummaryTask(customArgs: string[]): StringTask { +export function diffSummaryTask(customArgs: string[]): StringTask | EmptyTask { let logFormat = logFormatFromCommand(customArgs); const commands = ['diff']; @@ -16,17 +16,21 @@ export function diffSummaryTask(customArgs: string[]): StringTask { commands.push(...customArgs); - return { + return validateLogFormatConfig(commands) || { commands, format: 'utf-8', parser: getDiffParser(logFormat), - } + }; } -export function validateSummaryOptions(customArgs: unknown[]): EmptyTask | void { +export function validateLogFormatConfig(customArgs: unknown[]): EmptyTask | void { const flags = customArgs.filter(isLogFormat); if (flags.length > 1) { return configurationErrorTask(`Summary flags are mutually exclusive - pick one of ${flags.join(',')}`); } + + if (flags.length && customArgs.includes('-z')) { + return configurationErrorTask(`Summary flag ${flags} parsing is not compatible with null termination option '-z'`); + } } diff --git a/simple-git/src/lib/tasks/log.ts b/simple-git/src/lib/tasks/log.ts index 58a6a1cc..917597b6 100644 --- a/simple-git/src/lib/tasks/log.ts +++ b/simple-git/src/lib/tasks/log.ts @@ -1,5 +1,5 @@ -import { Options, StringTask } from '../types'; -import { LogResult, SimpleGit } from '../../../typings'; +import type { Options, StringTask } from '../types'; +import type { LogResult, SimpleGit } from '../../../typings'; import { logFormatFromCommand } from '../args/log-format'; import { COMMIT_BOUNDARY, @@ -18,7 +18,7 @@ import { } from '../utils'; import { SimpleGitApi } from '../simple-git-api'; import { configurationErrorTask } from './task'; -import { validateSummaryOptions } from './diff'; +import { validateLogFormatConfig } from './diff'; enum excludeOptions { '--pretty', @@ -150,8 +150,8 @@ export default function (): Pick { const next = trailingFunctionArgument(arguments); const options = parseLogOptions(trailingOptionsArgument(arguments), filterType(arguments[0], filterArray)); const task = rejectDeprecatedSignatures(...rest) || - validateSummaryOptions(options.commands) || - createLogTask(options) + validateLogFormatConfig(options.commands) || + createLogTask(options); return this._runTask(task, next); } diff --git a/simple-git/src/lib/tasks/stash-list.ts b/simple-git/src/lib/tasks/stash-list.ts index 4f0fd5cb..500cf44a 100644 --- a/simple-git/src/lib/tasks/stash-list.ts +++ b/simple-git/src/lib/tasks/stash-list.ts @@ -1,15 +1,17 @@ import { LogOptions, LogResult } from '../../../typings'; import { logFormatFromCommand } from '../args/log-format'; import { createListLogSummaryParser } from '../parsers/parse-list-log-summary'; -import { StringTask } from '../types'; +import type { StringTask } from '../types'; +import { validateLogFormatConfig } from './diff'; import { parseLogOptions } from './log'; +import type { EmptyTask } from './task'; -export function stashListTask(opt: LogOptions = {}, customArgs: string[]): StringTask { +export function stashListTask(opt: LogOptions = {}, customArgs: string[]): EmptyTask | StringTask { const options = parseLogOptions(opt); const commands = ['stash', 'list', ...options.commands, ...customArgs]; const parser = createListLogSummaryParser(options.splitter, options.fields, logFormatFromCommand(commands)); - return { + return validateLogFormatConfig(commands) || { commands, format: 'utf-8', parser, diff --git a/simple-git/src/lib/tasks/task.ts b/simple-git/src/lib/tasks/task.ts index 8bc572ff..713dcbcb 100644 --- a/simple-git/src/lib/tasks/task.ts +++ b/simple-git/src/lib/tasks/task.ts @@ -1,5 +1,5 @@ import { TaskConfigurationError } from '../errors/task-configuration-error'; -import { BufferTask, EmptyTaskParser, SimpleGitTask, StringTask } from '../types'; +import type { BufferTask, EmptyTaskParser, SimpleGitTask, StringTask } from '../types'; export const EMPTY_COMMANDS: [] = []; diff --git a/simple-git/src/lib/utils/task-parser.ts b/simple-git/src/lib/utils/task-parser.ts index 7b39628a..5be4690b 100644 --- a/simple-git/src/lib/utils/task-parser.ts +++ b/simple-git/src/lib/utils/task-parser.ts @@ -1,15 +1,15 @@ -import { TaskParser, TaskResponseFormat } from '../types'; +import type { MaybeArray, TaskParser, TaskResponseFormat } from '../types'; import { GitOutputStreams } from './git-output-streams'; import { LineParser } from './line-parser'; -import { toLinesWithContent } from './util'; +import { asArray, toLinesWithContent } from './util'; export function callTaskParser(parser: TaskParser, streams: GitOutputStreams) { return parser(streams.stdOut, streams.stdErr); } -export function parseStringResponse(result: T, parsers: LineParser[], ...texts: string[]): T { - texts.forEach(text => { - for (let lines = toLinesWithContent(text), i = 0, max = lines.length; i < max; i++) { +export function parseStringResponse(result: T, parsers: LineParser[], texts: MaybeArray, trim = true): T { + asArray(texts).forEach(text => { + for (let lines = toLinesWithContent(text, trim), i = 0, max = lines.length; i < max; i++) { const line = (offset = 0) => { if ((i + offset) >= max) { return; diff --git a/simple-git/test/integration/diff.spec.ts b/simple-git/test/integration/diff.spec.ts new file mode 100644 index 00000000..19ce1408 --- /dev/null +++ b/simple-git/test/integration/diff.spec.ts @@ -0,0 +1,44 @@ +import { + createTestContext, + like, + newSimpleGit, + setUpFilesAdded, + setUpInit, + SimpleGitTestContext +} from '../__fixtures__'; + +describe('diff', function () { + const nameWithTrailingSpaces = 'name-with-trailing-spaces '; + const fileContent = Array(10).fill('Some content on this line\n').join(''); + const nextContent = Array(5).fill('Some content on this line\nDifferent on this line\n').join(''); + + let context: SimpleGitTestContext; + + beforeEach(async () => { + context = await createTestContext(); + await setUpInit(context); + await setUpFilesAdded(context, [nameWithTrailingSpaces], '.', fileContent); + await context.file(nameWithTrailingSpaces, nextContent); + }); + + it('detects diff with --numstat', async () => { + const diff = await newSimpleGit(context.root).diffSummary(['--numstat']); + + expect(diff).toEqual(like({ + changed: 1, + deletions: 1, + insertions: 10, + files: [ + { + file: nameWithTrailingSpaces, + changes: 11, + insertions: 10, + deletions: 1, + binary: false, + } + ] + })); + }); + + +}); diff --git a/simple-git/test/integration/log.spec.ts b/simple-git/test/integration/log.spec.ts index b3b4555f..430c329d 100644 --- a/simple-git/test/integration/log.spec.ts +++ b/simple-git/test/integration/log.spec.ts @@ -1,3 +1,4 @@ +import { promiseResult } from '@kwsites/promise-result'; import { createTestContext, GIT_USER_EMAIL, @@ -8,6 +9,7 @@ import { setUpInit, SimpleGitTestContext } from '../__fixtures__'; +import type { DiffResultTextFile } from '../../typings'; describe('log', () => { let context: SimpleGitTestContext; @@ -50,10 +52,64 @@ describe('log', () => { ]); }); - it('should read one line for each commit when using shortstat', async () => { - const options = ['--shortstat']; - const actual = await newSimpleGit(context.root).log(options); + describe('log formats', () => { + const a = 'a.txt'; + const b = 'b.txt'; + + function file(file: string, changes = 0, insertions = 0, deletions = 0): DiffResultTextFile { + return { + file, + changes, + insertions, + deletions, + binary: false, + } + } + + function out(file: DiffResultTextFile, changed = 0, insertions = 0, deletions = 0) { + return { + diff: { + changed, + deletions, + insertions, + files: [file] + } + }; + } + + it('should read one line for each commit when using shortstat', async () => { + const options = ['--shortstat']; + const actual = await newSimpleGit(context.root).log(options); + + expect(actual.all).toHaveLength(2); + }); + + it('should work using numstat', async () => { + const options = ['--numstat']; + const actual = await newSimpleGit(context.root).log(options); + expect(actual).toEqual(like({ + all: [ + like(out(file(b,1, 1), 1, 1)), + like(out(file(a, 1, 1), 1, 1)), + ] + })) + }); + + it('should work name only (summary has count of file changes, files show no count data)', async () => { + const options = ['--name-only']; + const actual = await newSimpleGit(context.root).log(options); + expect(actual).toEqual(like({ + all: [ + like(out(file(b, 0), 1)), + like(out(file(a, 0), 1)), + ] + })) + }); + + it('should fail when using multiple summary types', async () => { + const result = await promiseResult(newSimpleGit(context.root).log(['--stat', '--numstat'])); - expect(actual.all).toHaveLength(2); + expect(result.threw).toBe(true); + }); }); }); diff --git a/simple-git/test/unit/diff.spec.ts b/simple-git/test/unit/diff.spec.ts index 6a03777b..904aaabb 100644 --- a/simple-git/test/unit/diff.spec.ts +++ b/simple-git/test/unit/diff.spec.ts @@ -1,3 +1,4 @@ +import { promiseError } from '@kwsites/promise-result'; import { assertExecutedCommands, assertGitError, @@ -12,7 +13,6 @@ import { import { SimpleGit, TaskConfigurationError } from '../..'; import { LogFormat } from '../../src/lib/args/log-format'; import { getDiffParser } from '../../src/lib/parsers/parse-diff-summary'; -import { promiseError } from '@kwsites/promise-result'; describe('diff', () => { @@ -244,4 +244,107 @@ describe('diff', () => { }); }); + describe('log-format', () => { + const file = 'simple-git/test/unit/diff.spec.ts'; + + beforeEach(() => git = newSimpleGit()); + + it('diffSummary with --numstat', async () => { + const task = git.diffSummary(['--numstat']); + await closeWithSuccess(`14\t0\t${file}\n`); + + assertExecutedCommands('diff', '--numstat'); + expect(await task).toEqual(like({ + changed: 1, + deletions: 0, + insertions: 14, + files: [ + { + file, + changes: 14, + insertions: 14, + deletions: 0, + binary: false, + } + ] + })) + }); + + it('diffSummary with custom --stat', async () => { + const task = git.diffSummary(['--foo', '--stat', 'bar']); + await closeWithSuccess(` + ${file} | 14 ++++++++++++++ + 1 file changed, 14 insertions(+) +`); + + assertExecutedCommands('diff', '--foo', '--stat', 'bar'); + expect(await task).toEqual(like({ + changed: 1, + deletions: 0, + insertions: 14, + files: [ + { + file, + changes: 14, + insertions: 14, + deletions: 0, + binary: false, + } + ] + })) + }); + + it('diffSummary with --name-only', async () => { + const task = git.diffSummary(['--name-only']); + await closeWithSuccess(file); + + assertExecutedCommands('diff', '--name-only'); + expect(await task).toEqual(like({ + changed: 1, + deletions: 0, + insertions: 0, + files: [ + { + file, + changes: 0, + insertions: 0, + deletions: 0, + binary: false, + } + ] + })) + }); + + it('diffSummary with --name-status', async () => { + const task = git.diffSummary(['--name-status']); + await closeWithSuccess(`M ${file}`); + + assertExecutedCommands('diff', '--name-status'); + expect(await task).toEqual(like({ + changed: 1, + deletions: 0, + insertions: 0, + files: [ + { + file, + changes: 0, + insertions: 0, + deletions: 0, + binary: false, + } + ] + })) + }); + + it('disallows multiple output formats', async () => { + const task = promiseError(git.diffSummary(['--stat', '--numstat'])); + assertGitError(await task, 'Summary flags are mutually exclusive'); + }); + + it('disallows null terminators when using a summary format parser', async () => { + const task = promiseError(git.diffSummary(['--name-only', '-z'])); + assertGitError(await task, 'Summary flag --name-only parsing is not compatible with null termination'); + }); + }); + });