Skip to content
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

Prefix padding #495

Merged
merged 6 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
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
9 changes: 9 additions & 0 deletions bin/concurrently.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,15 @@ describe('specifies custom prefix length', () => {
});
});

describe('--pad-prefix', () => {
it('pads prefixes with spaces', async () => {
const lines = await run('--pad-prefix -n foo,barbaz "echo foo" "echo bar"').getLogLines();

expect(lines).toContainEqual(expect.stringContaining('[foo ]'));
expect(lines).toContainEqual(expect.stringContaining('[barbaz]'));
});
});

describe('--restart-tries', () => {
it('changes how many times a command will restart', async () => {
const lines = await run('--restart-tries 1 "exit 1"').getLogLines();
Expand Down
7 changes: 6 additions & 1 deletion bin/concurrently.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ const args = yargs(argsBeforeSep)
default: defaults.prefixLength,
type: 'number',
},
'pad-prefix': {
describe: 'Pads short prefixes with spaces so that the length of all prefixes match',
type: 'boolean',
},
'timestamp-format': {
alias: 't',
describe: 'Specify the timestamp in moment/date-fns format.',
Expand Down Expand Up @@ -190,7 +194,7 @@ const args = yargs(argsBeforeSep)
['m', 'n', 'name-separator', 's', 'r', 'no-color', 'hide', 'g', 'timings', 'P'],
'General',
)
.group(['p', 'c', 'l', 't'], 'Prefix styling')
.group(['p', 'c', 'l', 't', 'pad-prefix'], 'Prefix styling')
.group(['i', 'default-input-target'], 'Input handling')
.group(['k', 'kill-others-on-fail', 'kill-signal'], 'Killing other processes')
.group(['restart-tries', 'restart-after'], 'Restarting')
Expand Down Expand Up @@ -223,6 +227,7 @@ concurrently(
prefix: args.prefix,
prefixColors: args.prefixColors.split(','),
prefixLength: args.prefixLength,
padPrefix: args.padPrefix,
restartDelay:
args.restartAfter === 'exponential' ? 'exponential' : Number(args.restartAfter),
restartTries: args.restartTries,
Expand Down
67 changes: 67 additions & 0 deletions src/flow-control/logger-padding.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import createMockInstance from 'jest-create-mock-instance';

import { FakeCommand } from '../fixtures/fake-command';
import { Logger } from '../logger';
import { LoggerPadding } from './logger-padding';

let logger: jest.Mocked<Logger>;
let controller: LoggerPadding;
let commands: FakeCommand[];

beforeEach(() => {
commands = [new FakeCommand(), new FakeCommand()];
logger = createMockInstance(Logger);
controller = new LoggerPadding({ logger });
});

it('returns same commands', () => {
expect(controller.handle(commands)).toMatchObject({ commands });
});

it('sets the prefix length on handle', () => {
controller.handle(commands);
expect(logger.setPrefixLength).toHaveBeenCalledTimes(1);
});

it('updates the prefix length when commands emit a start timer', () => {
controller.handle(commands);
commands[0].timer.next({ startDate: new Date() });
expect(logger.setPrefixLength).toHaveBeenCalledTimes(2);

commands[1].timer.next({ startDate: new Date() });
expect(logger.setPrefixLength).toHaveBeenCalledTimes(3);
});

it('sets prefix length to the longest prefix of all commands', () => {
logger.getPrefixContent
.mockReturnValueOnce({ type: 'default', value: 'foobar' })
.mockReturnValueOnce({ type: 'default', value: 'baz' });

controller.handle(commands);
expect(logger.setPrefixLength).toHaveBeenCalledWith(6);
});

it('does not shorten the prefix length', () => {
logger.getPrefixContent
.mockReturnValueOnce({ type: 'default', value: '100' })
.mockReturnValueOnce({ type: 'default', value: '1' });

controller.handle(commands);
commands[0].timer.next({ startDate: new Date() });
expect(logger.setPrefixLength).toHaveBeenCalledWith(3);

commands[0].timer.next({ startDate: new Date() });
expect(logger.setPrefixLength).toHaveBeenCalledWith(3);
});

it('unsubscribes from start timers on finish', () => {
logger.getPrefixContent.mockReturnValue({ type: 'default', value: '1' });

const { onFinish } = controller.handle(commands);
commands[0].timer.next({ startDate: new Date() });
expect(logger.setPrefixLength).toHaveBeenCalledTimes(2);

onFinish();
commands[0].timer.next({ startDate: new Date() });
expect(logger.setPrefixLength).toHaveBeenCalledTimes(2);
});
41 changes: 41 additions & 0 deletions src/flow-control/logger-padding.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { Command } from '../command';
import { Logger } from '../logger';
import { FlowController } from './flow-controller';

export class LoggerPadding implements FlowController {
private readonly logger: Logger;

constructor({ logger }: { logger: Logger }) {
this.logger = logger;
}

handle(commands: Command[]): { commands: Command[]; onFinish: () => void } {
// Sometimes there's limited concurrency, so not all commands will spawn straight away.
// Compute the prefix length now, which works for all styles but those with a PID.
let length = commands.reduce((length, command) => {
const content = this.logger.getPrefixContent(command);
return Math.max(length, content?.value.length || 0);
}, 0);
this.logger.setPrefixLength(length);

// The length of prefixes is somewhat stable, except for PIDs, which might change when a
// process spawns (e.g. PIDs might look like 1, 10 or 100), therefore listen to command starts
// and update the prefix length when this happens.
const subs = commands.map((command) =>
command.timer.subscribe((event) => {
if (!event.endDate) {
const content = this.logger.getPrefixContent(command);
length = Math.max(length, content?.value.length || 0);
this.logger.setPrefixLength(length);
}
}),
);

return {
commands,
onFinish() {
subs.forEach((sub) => sub.unsubscribe());
},
};
}
}
10 changes: 9 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { LogError } from './flow-control/log-error';
import { LogExit } from './flow-control/log-exit';
import { LogOutput } from './flow-control/log-output';
import { LogTimings } from './flow-control/log-timings';
import { LoggerPadding } from './flow-control/logger-padding';
import { RestartDelay, RestartProcess } from './flow-control/restart-process';
import { Logger } from './logger';

Expand All @@ -37,6 +38,11 @@ export type ConcurrentlyOptions = Omit<BaseConcurrentlyOptions, 'abortSignal' |
*/
prefixLength?: number;

/**
* Pads short prefixes with spaces so that all prefixes have the same length.
*/
padPrefix?: boolean;

/**
* Whether output should be formatted to include prefixes and whether "event" logs will be logged.
*/
Expand Down Expand Up @@ -103,7 +109,7 @@ export function concurrently(
const logger = new Logger({
hide,
prefixFormat: options.prefix,
prefixLength: options.prefixLength,
commandLength: options.prefixLength,
raw: options.raw,
timestampFormat: options.timestampFormat,
});
Expand All @@ -121,6 +127,8 @@ export function concurrently(
group: options.group,
abortSignal: abortController.signal,
controllers: [
// LoggerPadding needs to run before any other controllers that might output something
...(options.padPrefix ? [new LoggerPadding({ logger })] : []),
new LogError({ logger }),
new LogOutput({ logger }),
new LogExit({ logger }),
Expand Down
22 changes: 20 additions & 2 deletions src/logger.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,32 @@ describe('#logCommandText()', () => {
expect(logger.log).toHaveBeenCalledWith(chalk.reset('[echo foo]') + ' ', 'foo', cmd);
});

it('logs prefix using command line itself, capped at prefixLength bytes', () => {
const { logger } = createLogger({ prefixFormat: 'command', prefixLength: 6 });
it('logs prefix using command line itself, capped at commandLength bytes', () => {
const { logger } = createLogger({ prefixFormat: 'command', commandLength: 6 });
const cmd = new FakeCommand();
logger.logCommandText('foo', cmd);

expect(logger.log).toHaveBeenCalledWith(chalk.reset('[ec..oo]') + ' ', 'foo', cmd);
});

it('logs default prefixes with padding', () => {
const { logger } = createLogger({});
const cmd = new FakeCommand('foo');
logger.setPrefixLength(5);
logger.logCommandText('bar', cmd);

expect(logger.log).toHaveBeenCalledWith(chalk.reset('[foo ]') + ' ', 'bar', cmd);
});

it('logs templated prefixes with padding', () => {
const { logger } = createLogger({ prefixFormat: '{name}-{index}' });
const cmd = new FakeCommand('foo', undefined, 0);
logger.setPrefixLength(6);
logger.logCommandText('bar', cmd);

expect(logger.log).toHaveBeenCalledWith(chalk.reset('foo-0 ') + ' ', 'bar', cmd);
});

it('logs prefix using prefixColor from command', () => {
const { logger } = createLogger({});
const cmd = new FakeCommand('', undefined, 1, {
Expand Down
50 changes: 38 additions & 12 deletions src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@ export class Logger {
private readonly hide: CommandIdentifier[];
private readonly raw: boolean;
private readonly prefixFormat?: string;
private readonly prefixLength: number;
private readonly commandLength: number;
private readonly timestampFormat: string;

/**
* How many characters should a prefix have.
* Prefixes shorter than this will be padded with spaces to the right.
*/
private prefixLength = 0;

/**
* Last character emitted.
* If `undefined`, then nothing has been logged yet.
Expand All @@ -28,7 +34,7 @@ export class Logger {
constructor({
hide,
prefixFormat,
prefixLength,
commandLength,
raw = false,
timestampFormat,
}: {
Expand All @@ -50,9 +56,9 @@ export class Logger {
prefixFormat?: string;

/**
* How many characters should a prefix have at most, used when the prefix format is `command`.
* How many characters should a prefix have at most when the format is `command`.
*/
prefixLength?: number;
commandLength?: number;

/**
* Date format used when logging date/time.
Expand All @@ -63,17 +69,17 @@ export class Logger {
this.hide = (hide || []).map(String);
this.raw = raw;
this.prefixFormat = prefixFormat;
this.prefixLength = prefixLength || defaults.prefixLength;
this.commandLength = commandLength || defaults.prefixLength;
this.timestampFormat = timestampFormat || defaults.timestampFormat;
}

private shortenText(text: string) {
if (!text || text.length <= this.prefixLength) {
if (!text || text.length <= this.commandLength) {
return text;
}

const ellipsis = '..';
const prefixLength = this.prefixLength - ellipsis.length;
const prefixLength = this.commandLength - ellipsis.length;
const endLength = Math.floor(prefixLength / 2);
const beginningLength = prefixLength - endLength;

Expand All @@ -84,33 +90,53 @@ export class Logger {

private getPrefixesFor(command: Command): Record<string, string> {
return {
pid: String(command.pid),
// When there's limited concurrency, the PID might not be immediately available,
// so avoid the string 'undefined' from becoming a prefix
pid: command.pid != null ? String(command.pid) : '',
gustavohenke marked this conversation as resolved.
Show resolved Hide resolved
index: String(command.index),
name: command.name,
command: this.shortenText(command.command),
time: formatDate(Date.now(), this.timestampFormat),
};
}

getPrefix(command: Command) {
getPrefixContent(
command: Command,
): { type: 'default' | 'template'; value: string } | undefined {
const prefix = this.prefixFormat || (command.name ? 'name' : 'index');
if (prefix === 'none') {
return '';
return;
}

const prefixes = this.getPrefixesFor(command);
if (Object.keys(prefixes).includes(prefix)) {
return `[${prefixes[prefix]}]`;
return { type: 'default', value: prefixes[prefix] };
}

return _.reduce(
const value = _.reduce(
prefixes,
(prev, val, key) => {
const keyRegex = new RegExp(_.escapeRegExp(`{${key}}`), 'g');
return prev.replace(keyRegex, String(val));
},
prefix,
);
return { type: 'template', value };
}

getPrefix(command: Command): string {
const content = this.getPrefixContent(command);
if (!content) {
return '';
}

return content.type === 'template'
? content.value.padEnd(this.prefixLength, ' ')
: `[${content.value.padEnd(this.prefixLength, ' ')}]`;
}

setPrefixLength(length: number) {
this.prefixLength = length;
}

colorText(command: Command, text: string) {
Expand Down
Loading