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

Update to powershell grammars #52956

Closed
wants to merge 2 commits into from
Closed

Update to powershell grammars #52956

wants to merge 2 commits into from

Conversation

omniomi
Copy link
Contributor

@omniomi omniomi commented Jun 26, 2018

This is an incremental update from https://github.com/PowerShell/EditorSyntax

  • Handful of scope corrections and fixes.
  • One bug fix with comment documentation.

Our tests are passing: (https://ci.appveyor.com/project/powershell/editorsyntax/branch/master)

Running specs...

...

Finished in 21.437 seconds
367 tests, 3524 assertions, 0 failures, 0 skipped

as are the vscode-colorize-tests:

colorization                                            spec.js:40
...
√ powershell-test.ps1 (455ms)                           spec.js:67
...

@omniomi
Copy link
Contributor Author

omniomi commented Jul 2, 2018

@aeschli do you need anything from me to merge this update to the PowerShell grammar? The build failed because master was broken when I forked it. If you want me to merge in the current master I can.

@omniomi
Copy link
Contributor Author

omniomi commented Jul 7, 2018

@aeschli I merged the latest master and now the tests are failing. It seems to indicate 'source.powershell comment.line.powershell punctuation.definition.comment.powershell' was the issue but it doesn't indicate what it was expecting or what it got. On the latest version on insiders the comment opener # matches the test file using the latest syntax . https://vscode.visualstudio.com/VSCode/_build/results?buildId=3289&view=ms.vss-test-web.test-result-details

This PR also doesn't touch the comment scopes except for "comment.documentation.embedded.powershell"

Can you please point me at how I figure out what exactly your tests are doing?

Pinging @TylerLeonhardt into the loop.

@aeschli
Copy link
Contributor

aeschli commented Jul 9, 2018

I pushed a new commit with the updated grammar and tests.

@aeschli aeschli closed this Jul 9, 2018
@aeschli aeschli added this to the July 2018 milestone Jul 9, 2018
aeschli added a commit that referenced this pull request Jul 9, 2018
@aeschli
Copy link
Contributor

aeschli commented Jul 9, 2018

Thanks @omniomi. Instead of trying to fix merge issues, I suggest to just rerun the integration tests ( scripts/test-integration.sh(bat)). The test takes the entries in test/colorize-fixtures and runs the grammar and theme rule matching on it. The result is written to test/colorize-result. If the written result is identical to the previous result or contains only scope name change, the test is green, else it is red. Use the Git diff viewer to see the differences in the result. If all changes make sense, accept the new result by committing it.

@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants