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.

Sign up for GitHub

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

Don't choke on switches in --example=VALUE format #6759

Closed
wants to merge 6 commits into from
Closed

Don't choke on switches in --example=VALUE format #6759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2b072f8
Don't choke on switches in --example=VALUE format
Jan 30, 2016
1f45c43
Address pull review comments
Jan 30, 2016
279f15e
Don't shift values for boolean options into args
Jan 31, 2016
abad471
Address review comments for superficial changes
Feb 2, 2016
d3f75d8
Diagnostic for longform boolean flag given a value
Feb 2, 2016
47ed17a
Don't inject synthesized strings into command line
Feb 2, 2016
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
72 changes: 58 additions & 14 deletions src/compiler/commandLineParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,12 @@ namespace ts {
return optionNameMapCache;
}

interface ArgumentStream {
synthesizedArgs: string[];
commandLineArgs: string[];
cliPosition: number;
}

export function parseCommandLine(commandLine: string[], readFile?: (path: string) => string): ParsedCommandLine {
const options: CompilerOptions = {};
const fileNames: string[] = [];
Expand All @@ -363,21 +369,42 @@ namespace ts {
};

function parseStrings(args: string[]) {
let i = 0;
while (i < args.length) {
let s = args[i];
i++;
const strings: ArgumentStream = {
synthesizedArgs: [],
commandLineArgs: args,
cliPosition: 0
};
let s = getNextString(strings);
while (s !== undefined) {
if (s.charCodeAt(0) === CharacterCodes.at) {
parseResponseFile(s.slice(1));
}
else if (s.charCodeAt(0) === CharacterCodes.minus) {
s = s.slice(s.charCodeAt(1) === CharacterCodes.minus ? 2 : 1).toLowerCase();
s = s.slice(s.charCodeAt(1) === CharacterCodes.minus ? 2 : 1);

// Try to translate short option names to their full equivalents.
if (hasProperty(shortOptionNames, s)) {
s = shortOptionNames[s];
if (hasProperty(shortOptionNames, s.toLowerCase())) {
s = shortOptionNames[s.toLowerCase()];
}
else {
Copy link
Member

Choose a reason for hiding this comment

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

It's weird that this doesn't work for -t=es3 because you try to do the shorthand lookup first.

Copy link
Author

Choose a reason for hiding this comment

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

Right now tsc doesn't really follow anybody's conventions. It's more like an ad-hoc compromise between PowerShell (camelcase flags and space separators for values) and Linux (double dash flags like getopt_long uses).

Even though it's not documented, the way it's implemented allows you to go full PowerShell-style and use e.g., -init instead of --init. This is mostly about making the getopt_long way work, too. Is there any platform whose conventions match the -t=es3 format? Should I make that format allowed, too?

// When using long-form switches, we follow standard command-line conventions and accept
// "--example=VALUE", but we also accept "--example VALUE".
const optionName = s.split("=", 1)[0].toLowerCase();
if (optionName.length < s.length && hasProperty(optionNameMap, optionName)) {
// It's in "--example=VALUE" format. Separate the value and inject it into the arg stream
// so it's present for the next read.
const value = s.substring(optionName.length + 1);
const opt = optionNameMap[optionName];
if (opt.type === "boolean") {
errors.push(createCompilerDiagnostic(Diagnostics.Option_0_does_not_expect_a_parameter_but_was_given_1, opt.name, value));
continue;
}
strings.synthesizedArgs.unshift(value);
s = optionName;
}
}

s = s.toLowerCase();
if (hasProperty(optionNameMap, s)) {
const opt = optionNameMap[s];

Expand All @@ -386,27 +413,24 @@ namespace ts {
}
else {
// Check to see if no argument was provided (e.g. "--locale" is the last command-line argument).
if (!args[i] && opt.type !== "boolean") {
if (!getNextString(strings, /*holdPosition*/ true) && opt.type !== "boolean") {
errors.push(createCompilerDiagnostic(Diagnostics.Compiler_option_0_expects_an_argument, opt.name));
}

switch (opt.type) {
case "number":
options[opt.name] = parseInt(args[i]);
i++;
options[opt.name] = parseInt(getNextString(strings));
break;
case "boolean":
options[opt.name] = true;
break;
case "string":
options[opt.name] = args[i] || "";
i++;
options[opt.name] = getNextString(strings) || "";
break;
// If not a primitive, the possible types are specified in what is effectively a map of options.
default:
let map = <Map<number>>opt.type;
let key = (args[i] || "").toLowerCase();
i++;
let key = (getNextString(strings) || "").toLowerCase();
if (hasProperty(map, key)) {
options[opt.name] = map[key];
}
Expand All @@ -423,7 +447,27 @@ namespace ts {
else {
fileNames.push(s);
}

s = getNextString(strings);
}
}

function getNextString(stream: ArgumentStream, holdPosition = false) {
if (stream.synthesizedArgs.length > 0) {
const next = stream.synthesizedArgs[0];
if (!holdPosition) {
stream.synthesizedArgs.shift();
}

return next;
}

const next = stream.commandLineArgs[stream.cliPosition];
if (!holdPosition) {
stream.cliPosition++;
}

return next;
}

function parseResponseFile(fileName: string) {
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2187,6 +2187,10 @@
"category": "Error",
"code": 5062
},
"Option '{0}' does not expect a parameter, but was given '{1}'.": {
"category": "Error",
"code": 5063
},
"Concatenate and emit output to single file.": {
"category": "Message",
"code": 6001
Expand Down