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

Script side implementation for Brace Completion. #7587

Merged
merged 6 commits into from Apr 15, 2016
Merged

Conversation

paulvanbrenk
Copy link
Contributor

This needs updated Visual Studio components to work.

Fixes #1484

This needs updated Visual Studio components to work.
@@ -1844,6 +1844,31 @@ namespace FourSlash {
});
}

public verifyBraceCompletionAtPostion(negative: boolean, openingBrace: string) {

let charCode = 0x28; // '('
Copy link
Member

Choose a reason for hiding this comment

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

Use ts.CharacterCodes.

Copy link
Member

Choose a reason for hiding this comment

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

Just make perform the assignment in the switch to make it more uniform

case '"': charCode = ts.CharacterCodes.doubleQuote; break;
case "`": charCode = ts.CharacterCodes.backtick; break;
case "<": charCode = ts.CharacterCodes.lessThan; break;
}
Copy link
Member

Choose a reason for hiding this comment

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

Consider using a Map<CharacterCodes> and writing

const openBraceMap: Map<CharacterCodes> = {
    "{": CharacterCodes.openBrace,
     // etc.
}

Copy link
Member

Choose a reason for hiding this comment

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

You should error when the test-writer accidentally gave the wrong input.

@@ -1844,6 +1844,37 @@ namespace FourSlash {
});
}

public verifyBraceCompletionAtPostion(negative: boolean, openingBrace: string) {

const openBraceMap: ts.Map<ts.CharacterCodes> = {
Copy link
Member

Choose a reason for hiding this comment

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

For every call you're going to be creating this, so make this a static property

@DanielRosenwasser
Copy link
Member

There is some very bizarre handling here, but I am giving this a 👍 because users can always turn this setting off.

Can you open an infrastructure bug on what we've found here?

@paulvanbrenk paulvanbrenk merged commit 96deb55 into master Apr 15, 2016
@paulvanbrenk paulvanbrenk deleted the braceCompletion branch April 15, 2016 18:38
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants