Skip to content

Commit

Permalink
Use a required state object to contain the registration state of the …
Browse files Browse the repository at this point in the history
…parameters
  • Loading branch information
D4N14L committed Oct 27, 2023
1 parent d0c28e5 commit 1eca3a3
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 38 deletions.
10 changes: 6 additions & 4 deletions common/reviews/api/ts-command-line.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ export class AliasCommandLineAction extends CommandLineAction {
protected onExecute(): Promise<void>;
// @internal
_processParsedData(parserOptions: ICommandLineParserOptions, data: _ICommandLineParserData): void;
// Warning: (ae-forgotten-export) The symbol "IRegisterDefinedParametersState" needs to be exported by the entry point index.d.ts
//
// @internal (undocumented)
_registerDefinedParameters(existingParameterNames?: Set<string>): void;
_registerDefinedParameters(state: IRegisterDefinedParametersState): void;
readonly targetAction: CommandLineAction;
}

Expand Down Expand Up @@ -189,7 +191,7 @@ export abstract class CommandLineParameterProvider {
// (undocumented)
protected _registerAmbiguousParameter(name: string, parserKey: string): void;
// @internal (undocumented)
_registerDefinedParameters(existingParameterNames?: Set<string>): void;
_registerDefinedParameters(state: IRegisterDefinedParametersState): void;
// @internal (undocumented)
protected readonly _registeredParameterParserKeysByName: Map<string, string>;
// @internal (undocumented)
Expand Down Expand Up @@ -219,7 +221,7 @@ export abstract class CommandLineParser extends CommandLineParameterProvider {
protected _getArgumentParser(): argparse.ArgumentParser;
protected onExecute(): Promise<void>;
// @internal (undocumented)
_registerDefinedParameters(existingParameterNames?: Set<string>): void;
_registerDefinedParameters(state: IRegisterDefinedParametersState): void;
selectedAction: CommandLineAction | undefined;
tryGetAction(actionName: string): CommandLineAction | undefined;
}
Expand Down Expand Up @@ -389,7 +391,7 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
// @internal
_processParsedData(parserOptions: ICommandLineParserOptions, data: _ICommandLineParserData): void;
// @internal (undocumented)
_registerDefinedParameters(existingParameterNames?: Set<string>): void;
_registerDefinedParameters(state: IRegisterDefinedParametersState): void;
static readonly ScopingParameterGroup: typeof SCOPING_PARAMETER_GROUP;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as argparse from 'argparse';

import { CommandLineAction } from './CommandLineAction';
import { CommandLineParameterKind, type CommandLineParameter } from '../parameters/BaseClasses';
import type { ICommandLineParserData } from './CommandLineParameterProvider';
import type { ICommandLineParserData, IRegisterDefinedParametersState } from './CommandLineParameterProvider';
import type { ICommandLineParserOptions } from './CommandLineParser';
import type { CommandLineChoiceParameter } from '../parameters/CommandLineChoiceParameter';
import type { CommandLineFlagParameter } from '../parameters/CommandLineFlagParameter';
Expand Down Expand Up @@ -85,7 +85,7 @@ export class AliasCommandLineAction extends CommandLineAction {
}

/** @internal */
public _registerDefinedParameters(existingParameterNames?: Set<string>): void {
public _registerDefinedParameters(state: IRegisterDefinedParametersState): void {
/* override */
// All parameters are going to be defined by the target action. Re-use the target action parameters
// for this action.
Expand Down Expand Up @@ -153,8 +153,8 @@ export class AliasCommandLineAction extends CommandLineAction {
// Finally, register the parameters with the parser. We need to make sure that the target action
// is registered, since we need to re-use its parameters, and ambiguous parameters are discovered
// during registration. This will no-op if the target action is already registered.
this.targetAction._registerDefinedParameters(existingParameterNames);
super._registerDefinedParameters(existingParameterNames);
this.targetAction._registerDefinedParameters(state);
super._registerDefinedParameters(state);

// We need to re-map the ambiguous parameters after they are defined by calling
// super._registerDefinedParameters()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,19 @@ export interface IScopedLongNameParseResult {
scope: string | undefined;
}

/**
* An object containing the state of the
*
* @public
*/
export interface IRegisterDefinedParametersState {
/**
* A set of all defined parameter names registered by parent {@link CommandLineParameterProvider}
* objects.
*/
parentParameterNames: Set<string>;
}

/**
* This is the argparse result data object
* @internal
Expand Down Expand Up @@ -344,15 +357,21 @@ export abstract class CommandLineParameterProvider {
* Generates the command-line help text.
*/
public renderHelpText(): string {
this._registerDefinedParameters();
const initialState: IRegisterDefinedParametersState = {
parentParameterNames: new Set()
};
this._registerDefinedParameters(initialState);
return this._getArgumentParser().formatHelp();
}

/**
* Generates the command-line usage text.
*/
public renderUsageText(): string {
this._registerDefinedParameters();
const initialState: IRegisterDefinedParametersState = {
parentParameterNames: new Set()
};
this._registerDefinedParameters(initialState);
return this._getArgumentParser().formatUsage();
}

Expand Down Expand Up @@ -411,7 +430,7 @@ export abstract class CommandLineParameterProvider {
}

/** @internal */
public _registerDefinedParameters(existingParameterNames?: Set<string>): void {
public _registerDefinedParameters(state: IRegisterDefinedParametersState): void {
if (this._parametersHaveBeenRegistered) {
// We prevent new parameters from being defined after the first call to _registerDefinedParameters,
// so we can already ensure that all parameters were registered.
Expand Down Expand Up @@ -454,8 +473,9 @@ export abstract class CommandLineParameterProvider {

// Register the existing parameters as ambiguous parameters. These are generally provided by the
// parent action.
for (const existingParameterName of existingParameterNames || []) {
this._defineAmbiguousParameter(existingParameterName);
const { parentParameterNames } = state;
for (const parentParameterName of parentParameterNames) {
this._defineAmbiguousParameter(parentParameterName);
}

// We also need to loop through the defined ambiguous parameters and register them. These will be reported
Expand Down
27 changes: 20 additions & 7 deletions libraries/ts-command-line/src/providers/CommandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import colors from 'colors';

import type { CommandLineAction } from './CommandLineAction';
import type { AliasCommandLineAction } from './AliasCommandLineAction';
import { CommandLineParameterProvider, type ICommandLineParserData } from './CommandLineParameterProvider';
import {
CommandLineParameterProvider,
type IRegisterDefinedParametersState,
type ICommandLineParserData
} from './CommandLineParameterProvider';
import { CommandLineParserExitError, CustomArgumentParser } from './CommandLineParserExitError';
import { TabCompleteAction } from './TabCompletionAction';

Expand Down Expand Up @@ -202,7 +206,10 @@ export abstract class CommandLineParser extends CommandLineParameterProvider {
this._validateDefinitions();

// Register the parameters before we print help or parse the CLI
this._registerDefinedParameters();
const initialState: IRegisterDefinedParametersState = {
parentParameterNames: new Set()
};
this._registerDefinedParameters(initialState);

if (!args) {
// 0=node.exe, 1=script name
Expand Down Expand Up @@ -262,15 +269,21 @@ export abstract class CommandLineParser extends CommandLineParameterProvider {
}

/** @internal */
public _registerDefinedParameters(existingParameterNames?: Set<string>): void {
super._registerDefinedParameters(existingParameterNames);
public _registerDefinedParameters(state: IRegisterDefinedParametersState): void {
super._registerDefinedParameters(state);

const registeredParameterNames: Set<string> = new Set([
...(existingParameterNames || []),
const { parentParameterNames } = state;
const updatedParentParameterNames: Set<string> = new Set([
...parentParameterNames,
...this._registeredParameterParserKeysByName.keys()
]);

const parentState: IRegisterDefinedParametersState = {
...state,
parentParameterNames: updatedParentParameterNames
};
for (const action of this._actions) {
action._registerDefinedParameters(registeredParameterNames);
action._registerDefinedParameters(parentState);
}
}

Expand Down
47 changes: 31 additions & 16 deletions libraries/ts-command-line/src/providers/ScopedCommandLineAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@ import { CommandLineAction, type ICommandLineActionOptions } from './CommandLine
import { CommandLineParser, type ICommandLineParserOptions } from './CommandLineParser';
import { CommandLineParserExitError } from './CommandLineParserExitError';
import type { CommandLineParameter } from '../parameters/BaseClasses';
import type { CommandLineParameterProvider, ICommandLineParserData } from './CommandLineParameterProvider';
import type {
CommandLineParameterProvider,
ICommandLineParserData,
IRegisterDefinedParametersState
} from './CommandLineParameterProvider';

interface IInternalScopedCommandLineParserOptions extends ICommandLineParserOptions {
readonly actionOptions: ICommandLineActionOptions;
readonly unscopedActionParameters: ReadonlyArray<CommandLineParameter>;
readonly onDefineScopedParameters: (commandLineParameterProvider: CommandLineParameterProvider) => void;
readonly aliasAction?: string;
readonly aliasDocumentation?: string;
readonly existingParameterNames?: Set<string>;
readonly registerDefinedParametersState: IRegisterDefinedParametersState;
}

/**
Expand Down Expand Up @@ -59,8 +63,10 @@ class InternalScopedCommandLineParser extends CommandLineParser {
this._internalOptions.onDefineScopedParameters(this);
}

public _registerDefinedParameters(): void {
super._registerDefinedParameters(this._internalOptions.existingParameterNames);
public _registerDefinedParameters(state: IRegisterDefinedParametersState): void {
// Since we are in a separate parser, we need to register the parameters using the state
// from the parent parser.
super._registerDefinedParameters(this._internalOptions.registerDefinedParametersState);
}

protected async onExecute(): Promise<void> {
Expand Down Expand Up @@ -97,7 +103,7 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
private _scopingParameters: CommandLineParameter[];
private _unscopedParserOptions: ICommandLineParserOptions | undefined;
private _scopedCommandLineParser: InternalScopedCommandLineParser | undefined;
private _existingParameterNames: Set<string> = new Set();
private _subparserState: IRegisterDefinedParametersState | undefined;

/**
* The required group name to apply to all scoping parameters. At least one parameter
Expand Down Expand Up @@ -131,6 +137,12 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
// override
super._processParsedData(parserOptions, data);

// This should never happen because the super method should throw if parameters haven't been registered,
// but guard against this just in-case.
if (this._subparserState === undefined) {
throw new Error('Parameters have not been registered');
}

this._unscopedParserOptions = parserOptions;

// Generate the scoped parser using the parent parser information. We can only create this after we
Expand All @@ -141,7 +153,7 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
aliasAction: data.aliasAction,
aliasDocumentation: data.aliasDocumentation,
unscopedActionParameters: this.parameters,
existingParameterNames: this._existingParameterNames,
registerDefinedParametersState: this._subparserState,
onDefineScopedParameters: this.onDefineScopedParameters.bind(this)
});
}
Expand Down Expand Up @@ -190,16 +202,19 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
}

/** @internal */
public _registerDefinedParameters(existingParameterNames?: Set<string>): void {
super._registerDefinedParameters(existingParameterNames);

for (const registeredParameterName of this._registeredParameterParserKeysByName.keys()) {
this._existingParameterNames.add(registeredParameterName);
}

for (const existingParameterName of existingParameterNames || []) {
this._existingParameterNames.add(existingParameterName);
}
public _registerDefinedParameters(state: IRegisterDefinedParametersState): void {
super._registerDefinedParameters(state);

const { parentParameterNames } = state;
const updatedParentParameterNames: Set<string> = new Set([
...parentParameterNames,
...this._registeredParameterParserKeysByName.keys()
]);

this._subparserState = {
...state,
parentParameterNames: updatedParentParameterNames
};
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class TestCommandLine extends CommandLineParser {
describe(CommandLineParser.name, () => {
it('executes an action', async () => {
const commandLineParser: TestCommandLine = new TestCommandLine();
commandLineParser._registerDefinedParameters();
commandLineParser._registerDefinedParameters({ parentParameterNames: new Set() });

await commandLineParser.execute(['do:the-job', '--flag']);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function createParser(): DynamicCommandLineParser {
description: 'The action remainder'
});

commandLineParser._registerDefinedParameters();
commandLineParser._registerDefinedParameters({ parentParameterNames: new Set() });

return commandLineParser;
}
Expand Down

0 comments on commit 1eca3a3

Please sign in to comment.