Skip to content

Commit

Permalink
Allow negative numbers in options (#93)
Browse files Browse the repository at this point in the history
  • Loading branch information
Schniz authored May 24, 2021
1 parent eb01241 commit b58bbad
Show file tree
Hide file tree
Showing 15 changed files with 178 additions and 210 deletions.
23 changes: 23 additions & 0 deletions example/negative-numbers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { binary, command, run, number, option } from '../src';

export function createCmd() {
const cmd = command({
name: 'test',
args: {
number: option({
long: 'number',
type: number,
}),
},
async handler(args) {
console.log({ args });
},
});

return cmd;
}

if (require.main === module) {
const cmd = createCmd();
run(binary(cmd), process.argv);
}
15 changes: 15 additions & 0 deletions src/argparser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,24 @@ export type ParseContext = {

export type ParsingResult<Into> = Result<FailedParse, Into>;

/**
* A weird thing about command line interfaces is that they are not consistent without some context.
* Consider the following argument list: `my-app --server start`
*
* Should we parse it as `[positional my-app] [option --server start]`
* or should we parse it as `[positional my-app] [flag --server] [positional start]`?
*
* The answer is — it depends. A good command line utility has the context to know which key is a flag
* and which is an option that can take a value. We aim to be a good command line utility library, so
* we need to have the ability to provide this context.
*
* This is the small object that has this context.
*/
export type RegisterOptions = {
forceFlagLongNames: Set<string>;
forceFlagShortNames: Set<string>;
forceOptionLongNames: Set<string>;
forceOptionShortNames: Set<string>;
};

export type Register = {
Expand Down
9 changes: 7 additions & 2 deletions src/multioption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,20 @@ export function multioption<Decoder extends Type<string[], any>>(
},
];
},
register(_opts) {},
register(opts) {
opts.forceOptionLongNames.add(config.long);
if (config.short) {
opts.forceOptionShortNames.add(config.short);
}
},
async parse({
nodes,
visitedNodes,
}: ParseContext): Promise<ParsingResult<OutputOf<Decoder>>> {
const options = findOption(nodes, {
longNames: [config.long],
shortNames: config.short ? [config.short] : [],
}).filter(x => !visitedNodes.has(x));
}).filter((x) => !visitedNodes.has(x));

for (const option of options) {
visitedNodes.add(option);
Expand Down
53 changes: 18 additions & 35 deletions src/newparser/parser.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Token } from './tokenizer';
import createDebugger from 'debug';
import type { RegisterOptions } from '../argparser';

const debug = createDebugger('cmd-ts:parser');

Expand Down Expand Up @@ -46,44 +47,21 @@ interface ForcePositional extends BaseAstNode<'forcePositional'> {
type: 'forcePositional';
}

/**
* A weird thing about command line interfaces is that they are not consistent without some context.
* Consider the following argument list: `my-app --server start`
*
* Should we parse it as `[positional my-app] [option --server start]`
* or should we parse it as `[positional my-app] [flag --server] [positional start]`?
*
* The answer is — it depends. A good command line utility has the context to know which key is a flag
* and which is an option that can take a value. We aim to be a good command line utility library, so
* we need to have the ability to provide this context.
*
* This is the small object that has this context.
*/
type ForceFlag = {
/**
* Short keys that we will force to read as flags
*/
shortFlagKeys: Set<string>;
/**
* Long keys that we will force to read as flags
*/
longFlagKeys: Set<string>;
};

/**
* Create an AST from a token list
*
* @param tokens A token list, coming from `tokenizer.ts`
* @param forceFlag Keys to force as flag. {@see ForceFlag} to read more about it.
*/
export function parse(tokens: Token[], forceFlag: ForceFlag): AstNode[] {
export function parse(tokens: Token[], forceFlag: RegisterOptions): AstNode[] {
if (debug.enabled) {
debug(
`Registered short flags: ${JSON.stringify([...forceFlag.shortFlagKeys])}`
);
debug(
`Registered long flags: ${JSON.stringify([...forceFlag.longFlagKeys])}`
);
const registered = {
shortFlags: [...forceFlag.forceFlagShortNames],
longFlags: [...forceFlag.forceFlagLongNames],
shortOptions: [...forceFlag.forceOptionShortNames],
longOptions: [...forceFlag.forceOptionLongNames],
};
debug(`Registered:`, JSON.stringify(registered));
}

const nodes: AstNode[] = [];
Expand Down Expand Up @@ -162,9 +140,10 @@ export function parse(tokens: Token[], forceFlag: ForceFlag): AstNode[] {
const parsedValue = parseOptionValue({
key,
delimiterToken: nextToken,
forceFlag: forceFlag.longFlagKeys,
forceFlag: forceFlag.forceFlagLongNames,
getToken,
peekToken,
forceOption: forceFlag.forceOptionLongNames,
});
let raw = `--${key}`;

Expand Down Expand Up @@ -208,7 +187,8 @@ export function parse(tokens: Token[], forceFlag: ForceFlag): AstNode[] {
const parsedValue = parseOptionValue({
key: lastKey.raw,
delimiterToken: nextToken,
forceFlag: forceFlag.shortFlagKeys,
forceFlag: forceFlag.forceFlagShortNames,
forceOption: forceFlag.forceOptionShortNames,
getToken,
peekToken,
});
Expand Down Expand Up @@ -272,10 +252,13 @@ function parseOptionValue(opts: {
peekToken(): Token | undefined;
key: string;
forceFlag: Set<string>;
forceOption: Set<string>;
}): OptionValue | undefined {
let { getToken, delimiterToken, forceFlag, key } = opts;
let { getToken, delimiterToken, forceFlag, key, forceOption } = opts;
const shouldReadKeyAsOption = forceOption.has(key);
const shouldReadKeyAsFlag =
forceFlag.has(key) || opts.peekToken()?.type !== 'char';
!shouldReadKeyAsOption &&
(forceFlag.has(key) || opts.peekToken()?.type !== 'char');

if (!delimiterToken || (delimiterToken.raw !== '=' && shouldReadKeyAsFlag)) {
return;
Expand Down
7 changes: 6 additions & 1 deletion src/option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,12 @@ function fullOption<Decoder extends Type<string, any>>(
},
];
},
register(_opts) {},
register(opts) {
opts.forceOptionLongNames.add(config.long);
if (config.short) {
opts.forceOptionShortNames.add(config.short);
}
},
async parse({
nodes,
visitedNodes,
Expand Down
19 changes: 11 additions & 8 deletions src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,22 @@ export async function runSafely<R extends Runner<any, any>>(
ap: R,
strings: string[]
): Promise<Result<Exit, Into<R>>> {
const longFlagKeys = new Set<string>();
const shortFlagKeys = new Set<string>();
const longOptionKeys = new Set<string>();
const shortOptionKeys = new Set<string>();
const hotPath: string[] = [];
ap.register({
forceFlagShortNames: shortOptionKeys,
forceFlagLongNames: longOptionKeys,
});
const registerContext = {
forceFlagShortNames: shortFlagKeys,
forceFlagLongNames: longFlagKeys,
forceOptionShortNames: shortOptionKeys,
forceOptionLongNames: longOptionKeys,
};

ap.register(registerContext);

const tokens = tokenize(strings);
const nodes = parse(tokens, {
longFlagKeys: longOptionKeys,
shortFlagKeys: shortOptionKeys,
});
const nodes = parse(tokens, registerContext);

try {
const result = await ap.run({ nodes, visitedNodes: new Set(), hotPath });
Expand Down
80 changes: 28 additions & 52 deletions test/command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { parse } from '../src/newparser/parser';
import { command } from '../src/command';
import { number, string, boolean } from './test-types';
import * as Result from '../src/Result';
import { createRegisterOptions } from './createRegisterOptions';

const cmd = command({
name: 'My command',
Expand All @@ -18,26 +19,20 @@ const cmd = command({
}),
flag: flag({ type: boolean, long: 'flag' }),
},
handler: _ => {},
handler: (_) => {},
});

test('merges options, positionals and flags', async () => {
const argv = `first --option=666 second --second-option works-too --flag third`.split(
' '
);
const argv =
`first --option=666 second --second-option works-too --flag third`.split(
' '
);
const tokens = tokenize(argv);

const longOptionKeys = new Set<string>();
const shortOptionKeys = new Set<string>();
cmd.register({
forceFlagLongNames: longOptionKeys,
forceFlagShortNames: shortOptionKeys,
});
const registerOptions = createRegisterOptions();
cmd.register(registerOptions);

const nodes = parse(tokens, {
longFlagKeys: longOptionKeys,
shortFlagKeys: shortOptionKeys,
});
const nodes = parse(tokens, registerOptions);
const result = await cmd.parse({ nodes, visitedNodes: new Set() });
const expected: typeof result = Result.ok({
positionals: ['first', 'second', 'third'],
Expand All @@ -50,22 +45,17 @@ test('merges options, positionals and flags', async () => {
});

test('fails if an argument fail to parse', async () => {
const argv = `first --option=hello second --second-option works-too --flag=fails-too third`.split(
' '
);
const argv =
`first --option=hello second --second-option works-too --flag=fails-too third`.split(
' '
);
const tokens = tokenize(argv);

const longOptionKeys = new Set<string>();
const shortOptionKeys = new Set<string>();
cmd.register({
forceFlagLongNames: longOptionKeys,
forceFlagShortNames: shortOptionKeys,
});
const registerOptions = createRegisterOptions();

const nodes = parse(tokens, {
longFlagKeys: longOptionKeys,
shortFlagKeys: shortOptionKeys,
});
cmd.register(registerOptions);

const nodes = parse(tokens, registerOptions);
const result = cmd.parse({
nodes,
visitedNodes: new Set(),
Expand All @@ -75,11 +65,11 @@ test('fails if an argument fail to parse', async () => {
Result.err({
errors: [
{
nodes: nodes.filter(x => x.raw.startsWith('--option')),
nodes: nodes.filter((x) => x.raw.startsWith('--option')),
message: 'Not a number',
},
{
nodes: nodes.filter(x => x.raw.startsWith('--flag')),
nodes: nodes.filter((x) => x.raw.startsWith('--flag')),
message: `expected value to be either "true" or "false". got: "fails-too"`,
},
],
Expand All @@ -97,22 +87,15 @@ test('fails if providing unknown arguments', async () => {
args: {
positionals: restPositionals({ type: string }),
},
handler: _ => {},
handler: (_) => {},
});
const argv = `okay --option=failing alright --another=fail`.split(' ');
const tokens = tokenize(argv);

const longOptionKeys = new Set<string>();
const shortOptionKeys = new Set<string>();
cmd.register({
forceFlagLongNames: longOptionKeys,
forceFlagShortNames: shortOptionKeys,
});
const registerOptions = createRegisterOptions();
cmd.register(registerOptions);

const nodes = parse(tokens, {
longFlagKeys: longOptionKeys,
shortFlagKeys: shortOptionKeys,
});
const nodes = parse(tokens, registerOptions);
const result = await cmd.parse({
nodes,
visitedNodes: new Set(),
Expand All @@ -124,7 +107,7 @@ test('fails if providing unknown arguments', async () => {
{
message: 'Unknown arguments',
nodes: nodes.filter(
node =>
(node) =>
node.raw.startsWith('--option') ||
node.raw.startsWith('--another')
),
Expand All @@ -142,7 +125,7 @@ test('should fail run when an async handler fails', async () => {
const cmd = command({
name: 'my command',
args: {},
handler: async _ => {
handler: async (_) => {
throw error;
},
});
Expand All @@ -166,16 +149,9 @@ test('succeeds when rest is quoted', async () => {
'--',
'--restPositionals --trailing-comma all {{scripts,src}/**/*.{js,ts},{scripts,src}/*.{js,ts},*.{js,ts}}',
]);
const longOptionKeys = new Set<string>();
const shortOptionKeys = new Set<string>();
cmd.register({
forceFlagLongNames: longOptionKeys,
forceFlagShortNames: shortOptionKeys,
});
const nodes = parse(tokens, {
longFlagKeys: longOptionKeys,
shortFlagKeys: shortOptionKeys,
});
const registerOptions = createRegisterOptions();
cmd.register(registerOptions);
const nodes = parse(tokens, registerOptions);
const result = cmd.parse({
nodes,
visitedNodes: new Set(),
Expand Down
10 changes: 10 additions & 0 deletions test/createRegisterOptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import type { RegisterOptions } from '../src/argparser';

export function createRegisterOptions(): RegisterOptions {
return {
forceFlagLongNames: new Set(),
forceFlagShortNames: new Set(),
forceOptionLongNames: new Set(),
forceOptionShortNames: new Set(),
};
}
Loading

1 comment on commit b58bbad

@vercel
Copy link

@vercel vercel bot commented on b58bbad May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.