Skip to content

Including ` as part of foldEndPairCharacters#110108

Merged
mjbvz merged 2 commits intomicrosoft:masterfrom
sumanbk92:master
Nov 10, 2020
Merged

Including ` as part of foldEndPairCharacters#110108
mjbvz merged 2 commits intomicrosoft:masterfrom
sumanbk92:master

Conversation

@sumanbk92
Copy link
Contributor

@sumanbk92 sumanbk92 commented Nov 6, 2020 •
edited
Loading

This PR fixes folding lines incase of backticks (`)

@sumanbk92 sumanbk92 mentioned this pull request Nov 6, 2020
Closed
rheh
rheh reviewed Nov 6, 2020
View reviewed changes
Copy link

@rheh rheh left a comment

Choose a reason for hiding this comment

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

Should a new test be added to folding.test.ts?

@sumanbk92 sumanbk92 changed the title Including '`' as part of foldEndPairCharacters Including ` as part of foldEndPairCharacters Nov 6, 2020
@sumanbk92
Copy link
Contributor Author

sumanbk92 commented Nov 6, 2020

Should a new test be added to folding.test.ts?

Could you let me know how to run the folding.test.ts test file alone ?

@rheh
Copy link

rheh commented Nov 6, 2020

Should a new test be added to folding.test.ts?

Could you let me know how to run the folding.test.ts test file alone ?

I think this should help https://github.com/microsoft/vscode/blob/master/test/unit/README.md

sumanbk92
sumanbk92 commented Nov 6, 2020
View reviewed changes
Copy link
Contributor Author

@sumanbk92 sumanbk92 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Added Tests and folding.test.ts was not found under extensions/typescript-language-features/src/test.
Hence added appropriate test under extensions/html-language-features/server/src/test/folding.test.ts

@sumanbk92 sumanbk92 changed the title Including ` as part of foldEndPairCharacters https://github.com/microsoft/vscode/issues/110097 Including ` as part of foldEndPairCharacters Nov 6, 2020
@sumanbk92 sumanbk92 changed the title https://github.com/microsoft/vscode/issues/110097 Including ` as part of foldEndPairCharacters Including ` as part of foldEndPairCharacters Nov 6, 2020
sumanbk92
sumanbk92 commented Nov 7, 2020
View reviewed changes
Copy link
Contributor Author

@sumanbk92 sumanbk92 left a comment

Choose a reason for hiding this comment

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

@aeschli @doudou @threeve Please review this PR

@aeschli
Copy link
Contributor

aeschli commented Nov 9, 2020

The test extensions/html-language-features/server/src/test/folding.test.ts is not related to the code change in extensions/html-language-features/server/src/test/folding.test.ts. I suggest to remove the test.

@aeschli aeschli assigned mjbvz and unassigned aeschli Nov 9, 2020
@sumanbk92
Copy link
Contributor Author

sumanbk92 commented Nov 9, 2020

Removed unnecessary test.
Please review, @mjbvz

@mjbvz
Copy link
Collaborator

mjbvz commented Nov 9, 2020

Please create an issue the describe the problem this PR fixes. We need this for tracking what we fix each iteration

@mjbvz mjbvz added this to the November 2020 milestone Nov 9, 2020
@sumanbk92
Copy link
Contributor Author

sumanbk92 commented Nov 10, 2020

Please create an issue the describe the problem this PR fixes. We need this for tracking what we fix each iteration

The issue was created already. It is being tagged/referenced above in this PR.

Link: #110097

@mjbvz mjbvz merged commit 4247b29 into microsoft:master Nov 10, 2020
@mjbvz
Copy link
Collaborator

mjbvz commented Nov 10, 2020

Thanks. Will be in the next VS Code insiders build

In the future, make sure include the issue number in the PR issue description (or the commit message using fixes #110097 so that merging the PR closes the issue too)

@github-actions github-actions bot locked and limited conversation to collaborators Dec 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@rheh rheh rheh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mjbvz mjbvz

Labels

None yet

Projects

None yet

Milestone

November 2020

Development

Successfully merging this pull request may close these issues.

4 participants

@sumanbk92 @rheh @aeschli @mjbvz