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

[typescript-language-features] Add formatter option for semicolons #80828

Merged
merged 3 commits into from Sep 13, 2019

Conversation

@andrewbranch
Copy link
Member

commented Sep 12, 2019

Counterpart to microsoft/TypeScript#33402

Build will fail because I’m referencing the updated protocol from the TS PR—not sure how you handle proto updates but this will have to wait at least until the TS PR is merged.

image

Copy link
Contributor

left a comment

LGTM. To get testing, I'd like to merge this before we actually start shipping 3.7 so let's avoid using the new types

@@ -165,6 +165,7 @@ export default class FileConfigurationManager extends Disposable {
insertSpaceAfterTypeAssertion: config.get<boolean>('insertSpaceAfterTypeAssertion'),
placeOpenBraceOnNewLineForFunctions: config.get<boolean>('placeOpenBraceOnNewLineForFunctions'),
placeOpenBraceOnNewLineForControlBlocks: config.get<boolean>('placeOpenBraceOnNewLineForControlBlocks'),
semicolons: config.get<Proto.SemicolonPreference>('semicolons'),

This comment has been minimized.

Copy link
@mjbvz

mjbvz Sep 13, 2019

Contributor

So that we can merge this before VS Code includes TS 3.7, just use a normal string for now (or inline the string literal types)

This comment has been minimized.

Copy link
@andrewbranch

andrewbranch Sep 13, 2019

Author Member

Done. I had to modify the type of the object to get away with adding the additional property. Left a note that it can be removed after 3.7 is adopted.

@andrewbranch

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2019

Flaky Windows build?

@mjbvz mjbvz merged commit 8774e0b into microsoft:master Sep 13, 2019
1 of 2 checks passed
1 of 2 checks passed
VS Code #20190913.107 failed
Details
license/cla All CLA requirements met.
Details
@mjbvz

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2019

Yep, failure seems unrelated. Thanks for the PR!

@mjbvz mjbvz added this to the September 2019 milestone Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.