(GH-1413) Update grammar parsing for vscode-textmate v4 module #1414
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the logging object had no interface which made testing difficult. This commit adds a new interface which new features can use, which makes mocking the logging object possible.
3 tasks
|
Ping @rkeithhill |
VS Code 1.25 has updated the imported vsode-textmate module from v3 to v4, which introduced some breaking changes. This commit updates the grammar loader; * Uses a thenable grammar parser, as V4 of the module is all asynchronous * Uses a simple method detection technique on the Registry object to determine if we're using v3 or v4+ of the textmate module, and then branch the code accordingly. Note that this functionality can only be tested manually due to how the vscode-textmate module is incorporated into VSCode and the test-suite.
glennsarti
force-pushed
the
gh-1413-fix-for-textmate-v4
branch
from
fe2f631 to
de00cf3
Compare
rkeithhill
reviewed
Jul 9, 2018
src/features/Folding.ts
Outdated
| @@ -1,15 +1,15 @@ | |||
| /*--------------------------------------------------------- | |||
| * Copyright (C) Microsoft Corporation. All rights reserved. | |||
| *--------------------------------------------------------*/ | |||
|
|
|||
| import fs = require("fs"); | |||
There was a problem hiding this comment.
I think most of the rest of this codebase has a blank line after the copyright. Other than that LGTM. Thanks for fixing this!!
rkeithhill
approved these changes
Jul 9, 2018
|
BTW would be good to get this PR merged soon as I have several PRs waiting on a clean run through the tests. :-) |
glennsarti
changed the title
glennsarti
changed the title
|
Removed WIP tag. The blank line can be fixed later |
rjmholt
approved these changes
Jul 9, 2018
There was a problem hiding this comment.
LGTM! I was about to do a full style review, but if @rkeithhill agrees, it might be better just to merge this and deal with style inconsistencies later
Add newline between copyright header and body
|
@glennsarti @rkeithhill I just added that newline. Will wait for your (second) approval to merge |
|
@rjmholt Do you still have VSCode 1.24 installed? If so, could you test this on VSCode 1.24 to make sure the "conditional" code works? |
|
Looks like I can fold functions, |
glennsarti
added a commit
to glennsarti/vscode-powershell
that referenced
this pull request
Jul 10, 2018
Previously in PR PowerShell#1414 the Folding provider was updated, however the promise resolution was incorrect. This caused the grammar load to never trigger the Then command. This commit instead uses the correct way to return the value for the promise and, when then triggers the Then statement correctly.
3 tasks
rjmholt
pushed a commit
that referenced
this pull request
Jul 10, 2018
Previously in PR #1414 the Folding provider was updated, however the promise resolution was incorrect. This caused the grammar load to never trigger the Then command. This commit instead uses the correct way to return the value for the promise and, when then triggers the Then statement correctly.
rjmholt
pushed a commit
that referenced
this pull request
Jul 11, 2018
* (maint) Add a logging interface Previously the logging object had no interface which made testing difficult. This commit adds a new interface which new features can use, which makes mocking the logging object possible. * (GH-1413) Update grammar parsing for vscode-textmate v4 module VS Code 1.25 has updated the imported vsode-textmate module from v3 to v4, which introduced some breaking changes. This commit updates the grammar loader; * Uses a thenable grammar parser, as V4 of the module is all asynchronous * Uses a simple method detection technique on the Registry object to determine if we're using v3 or v4+ of the textmate module, and then branch the code accordingly. Note that this functionality can only be tested manually due to how the vscode-textmate module is incorporated into VSCode and the test-suite. * Update Folding.ts Add newline between copyright header and body
rjmholt
pushed a commit
that referenced
this pull request
Jul 11, 2018
Previously in PR #1414 the Folding provider was updated, however the promise resolution was incorrect. This caused the grammar load to never trigger the Then command. This commit instead uses the correct way to return the value for the promise and, when then triggers the Then statement correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Summary
VS Code 1.25 has updated the imported vsode-textmate module from v3 to v4, which
introduced some breaking changes. This commit updates the grammar loader;
Uses a thenable grammar parser, as V4 of the module is all asynchronous
Uses a simple method detection technique on the Registry object to determine
if we're using v3 or v4+ of the textmate module, and then branch the code
accordingly.
Note that this functionality can only be tested manually due to how the
vscode-textmate module is incorporated into VSCode and the test-suite.
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets.Please mark anything not applicable to this PR
NA.WIP:to the beginning of the title and remove the prefix when the PR is ready