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

Use a symbol for the 'scoping' parameter group #2

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
6 changes: 3 additions & 3 deletions common/reviews/api/ts-command-line.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ export abstract class CommandLineParameter {
readonly environmentVariable: string | undefined;
// @internal
_getSupplementaryNotes(supplementaryNotes: string[]): void;
readonly groupName: string | undefined;
abstract get kind(): CommandLineParameterKind;
readonly longName: string;
readonly parameterGroup: string | typeof SCOPING_PARAMETER_GROUP | undefined;
iclanton marked this conversation as resolved.
Show resolved Hide resolved
// @internal
_parserKey: string | undefined;
protected reportInvalidData(data: any): never;
Expand Down Expand Up @@ -253,7 +253,7 @@ export class DynamicCommandLineParser extends CommandLineParser {
export interface IBaseCommandLineDefinition {
description: string;
environmentVariable?: string;
parameterGroupName?: string;
parameterGroup?: string | typeof SCOPING_PARAMETER_GROUP;
parameterLongName: string;
parameterShortName?: string;
required?: boolean;
Expand Down Expand Up @@ -345,7 +345,7 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
get parameters(): ReadonlyArray<CommandLineParameter>;
// @internal
_processParsedData(parserOptions: ICommandLineParserOptions, data: _ICommandLineParserData): void;
static ScopingParameterGroupName: 'scoping';
static readonly ScopingParameterGroup: typeof SCOPING_PARAMETER_GROUP;
}

```
7 changes: 4 additions & 3 deletions libraries/ts-command-line/src/parameters/BaseClasses.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { SCOPING_PARAMETER_GROUP } from '../providers/ScopedCommandLineAction';
import { IBaseCommandLineDefinition, IBaseCommandLineDefinitionWithArgument } from './CommandLineDefinition';

/**
Expand Down Expand Up @@ -53,8 +54,8 @@ export abstract class CommandLineParameter {
/** {@inheritDoc IBaseCommandLineDefinition.parameterShortName} */
public readonly shortName: string | undefined;

/** {@inheritDoc IBaseCommandLineDefinition.parameterGroupName} */
public readonly groupName: string | undefined;
/** {@inheritDoc IBaseCommandLineDefinition.parameterGroup} */
public readonly parameterGroup: string | typeof SCOPING_PARAMETER_GROUP | undefined;

/** {@inheritDoc IBaseCommandLineDefinition.description} */
public readonly description: string;
Expand All @@ -72,7 +73,7 @@ export abstract class CommandLineParameter {
public constructor(definition: IBaseCommandLineDefinition) {
this.longName = definition.parameterLongName;
this.shortName = definition.parameterShortName;
this.groupName = definition.parameterGroupName;
this.parameterGroup = definition.parameterGroup;
this.description = definition.description;
this.required = !!definition.required;
this.environmentVariable = definition.environmentVariable;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { SCOPING_PARAMETER_GROUP } from '../providers/ScopedCommandLineAction';

/**
* For use with CommandLineParser, this interface represents a generic command-line parameter
*
Expand All @@ -20,7 +22,7 @@ export interface IBaseCommandLineDefinition {
/**
* An optional parameter group name, shown when invoking the tool with "--help"
*/
parameterGroupName?: string;
parameterGroup?: string | typeof SCOPING_PARAMETER_GROUP;

/**
* Documentation for the parameter that will be shown when invoking the tool with "--help"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { CommandLineFlagParameter } from '../parameters/CommandLineFlagParameter
import { CommandLineStringParameter } from '../parameters/CommandLineStringParameter';
import { CommandLineStringListParameter } from '../parameters/CommandLineStringListParameter';
import { CommandLineRemainder } from '../parameters/CommandLineRemainder';
import { SCOPING_PARAMETER_GROUP } from './ScopedCommandLineAction';

/**
* This is the argparse result data object
Expand All @@ -48,16 +49,16 @@ export abstract class CommandLineParameterProvider {

private _parameters: CommandLineParameter[];
private _parametersByLongName: Map<string, CommandLineParameter>;
private _parameterGroupsByName: Map<string, argparse.ArgumentGroup>;
private _parameterGroupsByName: Map<string | typeof SCOPING_PARAMETER_GROUP, argparse.ArgumentGroup>;
private _parametersProcessed: boolean;
private _remainder: CommandLineRemainder | undefined;

/** @internal */
// Third party code should not inherit subclasses or call this constructor
public constructor() {
this._parameters = [];
this._parametersByLongName = new Map<string, CommandLineParameter>();
this._parameterGroupsByName = new Map<string, argparse.ArgumentGroup>();
this._parametersByLongName = new Map();
this._parameterGroupsByName = new Map();
this._parametersProcessed = false;
}

Expand Down Expand Up @@ -448,13 +449,22 @@ export abstract class CommandLineParameterProvider {
}

let argumentGroup: argparse.ArgumentGroup | undefined;
if (parameter.groupName) {
argumentGroup = this._parameterGroupsByName.get(parameter.groupName);
if (parameter.parameterGroup) {
argumentGroup = this._parameterGroupsByName.get(parameter.parameterGroup);
if (!argumentGroup) {
let parameterGroupName: string;
if (typeof parameter.parameterGroup === 'string') {
parameterGroupName = parameter.parameterGroup;
} else if (parameter.parameterGroup === SCOPING_PARAMETER_GROUP) {
parameterGroupName = 'scoping';
} else {
throw new Error('Unexpected parameter group: ' + parameter.parameterGroup);
}

argumentGroup = this._getArgumentParser().addArgumentGroup({
title: `Optional ${parameter.groupName} arguments`
title: `Optional ${parameterGroupName} arguments`
});
this._parameterGroupsByName.set(parameter.groupName, argumentGroup);
this._parameterGroupsByName.set(parameter.parameterGroup, argumentGroup);
}
} else {
argumentGroup = this._getArgumentParser();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class InternalScopedCommandLineParser extends CommandLineParser {
}
}

export const SCOPING_PARAMETER_GROUP: unique symbol = Symbol('scoping');
iclanton marked this conversation as resolved.
Show resolved Hide resolved

/**
* Represents a sub-command that is part of the CommandLineParser command-line.
* Applications should create subclasses of ScopedCommandLineAction corresponding to
Expand Down Expand Up @@ -79,7 +81,7 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
* The required group name to apply to all scoping parameters. At least one parameter
* must be defined with this group name.
*/
public static ScopingParameterGroupName: 'scoping' = 'scoping';
public static readonly ScopingParameterGroup: typeof SCOPING_PARAMETER_GROUP = SCOPING_PARAMETER_GROUP;

public constructor(options: ICommandLineActionOptions) {
super(options);
Expand Down Expand Up @@ -200,7 +202,7 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
/** @internal */
protected _defineParameter(parameter: CommandLineParameter): void {
super._defineParameter(parameter);
if (parameter.groupName === ScopedCommandLineAction.ScopingParameterGroupName) {
if (parameter.parameterGroup === ScopedCommandLineAction.ScopingParameterGroup) {
this._scopingParameters.push(parameter);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class TestScopedAction extends ScopedCommandLineAction {

this._scopeArg = this.defineStringParameter({
parameterLongName: '--scope',
parameterGroupName: ScopedCommandLineAction.ScopingParameterGroupName,
parameterGroup: ScopedCommandLineAction.ScopingParameterGroup,
argumentName: 'SCOPE',
description: 'The scope'
});
Expand Down