-
Notifications
You must be signed in to change notification settings - Fork 81
Syntax bug fixes (#68, #77, #84, Range Operator) #100
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
Conversation
@@ -234,7 +234,7 @@ | |||
</dict> | |||
<dict> | |||
<key>match</key> | |||
<string>(?<!\w)-([ci]?[lg][te]|eq|ne)</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This incorrectly allows -lexx
Why is the build failing? On the commits above, it looks like it's failing on the last commit (2f400dd), but it did the same thing the last pull request I tried (#91), and this is the same thing minus that pull's last commit. The changes are fairly simple. Did I miss something, or is there something wrong with the test? |
It looks like this is failing because of the issue from #96. The build fails on |
I merged #96. It may be related. We already have a bunch of similar problem https://github.com/SublimeText/PowerShell/blob/master/tests/pester/SyntaxHelper.psm1#L139 |
Please, include samples of syntax that used to be incorrect and now correct to |
@@ -522,7 +528,7 @@ | |||
<key>function</key> | |||
<dict> | |||
<key>begin</key> | |||
<string>((?i:function|configuration|workflow))\s+((?:\p{L}|\d|_|-|\.)+)</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrectly treats a parameter -Function
as a function definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool
function echo([string]$text) { | ||
write-host $text | ||
} | ||
|
||
$file = join-path $env:SystemDrive "$([System.io.path]::GetRandomFileName()).ps1" | ||
$ScriptBlock | Out-File $file -Force | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, revert. These are changes I added recently, did you have problems with merge from dev?
@bcrotty I probably should write-up how to run tests locally. |
(Or whether some previous step is needed at all; I thought so.) |
@vors Sorry about the test-file. Should be sorted out now. |
no problem. I added instruction about local testing https://github.com/SublimeText/PowerShell/blob/dev/CONTRIBUTING.md#run-tests-locally |
For now, you can unblock yourself with adding failed scopes to https://github.com/SublimeText/PowerShell/blob/dev/tests/pester/SyntaxHelper.psm1#L139 |
Or create a separate |
@vors I might need some help. I added exclusions for |
Sorry, I mislead you a little bit. |
Ok, I'll work on that. Also, @vors I can't run the tests locally. I get the below error. From the source code, it looks like |
This code leaves in https://github.com/randy3k/UnitTesting/blob/master/sbin/appveyor.ps1 |
I finally got it working, but none of the I finally was able to run the test using the command: It looks like it copied PowerShell from the current directory that my PowerShell console was pointed to, which happened to be correct, but if I had wanted it to run on a different copy of the PowerShell code, I'm not sure how I would have done that. |
You are absolutely right.
I think the right way would be contribute to https://github.com/randy3k/UnitTesting/blob/master/sbin and provide two scripts: for local run (?) and for appvoyer run. |
Oh we need to merge this. Can you resolve merge conflicts? |
I think that's why this never got merged in the first place. I'm pretty sure the commits themselves are good, but we need to add an exclusion to the test, and I could use some help with that. |
Current tests framework is super complicated, involves cloning UnitTests repo, installing sublime texts, run python in-proc tests, serialize sublime scopes. I believe it can be replaced with scopes generation out-of-proc (maybe github library for that). It would greatly simplify troubleshooting. |
Just a heads up that a new syntax def using the new format .sublime-syntax is under work: The .sublime-syntax also includes the ability to test syntax defs. We need to integrate that with the tests in the PS package, though. I think fixing bugs in the .tmLanguage is ok (as it will help GH hilite PS files better too), but I'm not so sure we should spend too much time improving tests, etc. around the old format? |
No description provided.