Skip to content

fix: #55946 Added comment snippet variable#63572

Merged
jrieken merged 4 commits intomicrosoft:masterfrom
karanisverma:dev
Nov 22, 2018
Merged

fix: #55946 Added comment snippet variable#63572
jrieken merged 4 commits intomicrosoft:masterfrom
karanisverma:dev

Conversation

@karanisverma
Copy link
Contributor

@karanisverma karanisverma commented Nov 21, 2018 •
edited
Loading

Added comment snippet variable
it fixes following issue
#55946

@msftclas
Copy link

msftclas commented Nov 21, 2018 •
edited
Loading

CLA assistant check
All CLA requirements met.

jrieken
jrieken reviewed Nov 21, 2018
View reviewed changes
src/vs/editor/contrib/snippet/snippetVariables.ts Outdated
} else if (name === 'BLOCK_COMMENT_START') {
return comments.blockCommentStartToken;
} else if (name === 'BLOCK_COMMENT_END') {
return comments.blockCommentEndToken;
Copy link
Member

@jrieken jrieken Nov 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

... || undefined to ensure the default value is applied (in all cases)

Copy link
Contributor Author

@karanisverma karanisverma Nov 21, 2018

Choose a reason for hiding this comment

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

Thanks, I have made changes as per your suggestion.

@jrieken jrieken self-assigned this Nov 21, 2018
@jrieken jrieken added this to the November 2018 milestone Nov 21, 2018
@jrieken
Copy link
Member

jrieken commented Nov 21, 2018 •
edited
Loading

@karanisverma The build has failed due to a strict null error. You can them locally via F1 > Run Task > Strict Null Checks - it will then squiggle the code for null-violations

@karanisverma
Copy link
Contributor Author

karanisverma commented Nov 21, 2018

Thanks for the info, Looking into it :)

@karanisverma
Copy link
Contributor Author

karanisverma commented Nov 21, 2018

@jrieken I have fixed the issue and I have checked locally by following the steps you mentioned above.
Sharing log for test below:

> Executing task: npm run strict-null-check <


> [email protected] strict-null-check /Users/karanverma/me/oss/vscode
> tsc -p src/tsconfig.strictNullChecks.json


Terminal will be reused by tasks, press any key to close it.

build process seems to fail in following file
src/vs/base/browser/ui/tree/abstractTree.ts(205,12): error TS2532: Object is possibly 'undefined'.

I guess it's not because of the changes in this PR. Please let me know if there is anything I have to fix at my end.

@karanisverma
Copy link
Contributor Author

karanisverma commented Nov 22, 2018

@jrieken I have pulled code from master, tests are passing now 🙌🏼

jrieken
jrieken approved these changes Nov 22, 2018
View reviewed changes
Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

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

lgtm

@jrieken jrieken merged commit f325118 into microsoft:master Nov 22, 2018
@jrieken
Copy link
Member

jrieken commented Nov 22, 2018

Thanks @karanisverma

@karanisverma
Copy link
Contributor Author

karanisverma commented Nov 22, 2018

Anytime :), Thanks you for guiding me 🙂

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

Reviewers

@jrieken jrieken jrieken approved these changes

Assignees

@jrieken jrieken

Labels

None yet

Projects

None yet

Milestone

November 2018

Development

Successfully merging this pull request may close these issues.

3 participants

@karanisverma @msftclas @jrieken