Skip to content

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

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

Closed
wants to merge 6 commits into from
Closed

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

Changes from 1 commit
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
16 changes: 14 additions & 2 deletions src/compiler/commandLineParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,25 @@ namespace ts {
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)) {
if (hasProperty(shortOptionNames, s.toLowerCase())) {
s = shortOptionNames[s];
Copy link
Member

Choose a reason for hiding this comment

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

This no longer works correctly because shortOptionNames uses lowercase keys

Copy link
Author

Choose a reason for hiding this comment

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

Example: tsc -V will throw an error

Got it.

}
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".
var [ longFormSwitch ] = s.split("=", 1);
if (longFormSwitch.length < s.length && hasProperty(optionNameMap, longFormSwitch.toLowerCase())) {
// It's in "--example=VALUE" format. Replace it in the arg list with the separated format.
var value = s.substring(longFormSwitch.length + 1);
args.splice(i - 1, 1, longFormSwitch, value);
s = longFormSwitch;
}
}

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

Expand Down