Misc fixes/changes

[gravejester]

Fixes small bugs in the syntax highlighting rules. See commits for details.

[daviwil]

Dude, this is amazing! Thanks a lot for fixing all of these issues. I forgot to get the contributor licensing automation set up on this repo, need to get someone to hook that up before I can accept the PR.

In the meantime, did you happen to create any example script file for testing these changes? It might be nice to add an examples folder with a script (or scripts) that demonstrate the syntax that the highlighter should work correctly for. This will help us determine in the future whether we've broken the things that you've fixed here.

Thanks a lot for doing this work!

[gerane]

This is awesome! I totally agree with David on the examples.

[gravejester]

Thanks @daviwil. I'm working on some of the more complicated stuff as well.

I have also started on an example file. Will finish that and get it uploaded to the repo. I won't be able to get all the cases, so hopefully others can help out writing edge-cases in this file so that I have something to test against while fiddling with complicated regexes :)

[Jaykul]

What happened to the old example file(s)?

[daviwil]

Looks like the test files @Jaykul is referring to are here:

https://github.com/SublimeText/PowerShell/tree/dev/tests/samples

Might be helpful to pull those over also.

[gravejester]

@daviwil Nice, I will do that :) How is it going with getting the contributor licensing automation set up?

[daviwil]

Still waiting on someone to turn it on for me, I'll ping them again tomorrow (everyone's out for a holiday today).

[gravejester]

Ah.. sorry about that. There is no rush, I'm still tackling some of the more complicated stuff, and now I have a new test-file to test against as well.. will probably notice some stuff that needs fixing/tweaking :)

[vors]

That is awesome!

I tried to use lightshow, mentioned in #2 to test your changes

You can also do the same:

Current highlighting vs @gravejester highlighting for the sample.

I used split screen and scroll both windows more-or-less in sync (is there a better way?). On the screenshots bellow left side is original, right is updated syntax

Here are differences (both improvements and regressions) that I noticed:

• Node is now a keyword in Configuration, cool!
• Get-Foo -Param now highlights -Param with red. I think it's good But, it also introduce a regression: in the same context -foobar used to highlighted differently from -contains and other built-in operations.

• -filter highlighted wrong, but it's pretty obscure case about multi-line interpolations in here-string. It's kind-of broken anyway.

• Regression in complicated numeric constants (i.e. -0x34adMB)

[gravejester]

Nice @vors. Gonna have to play around with using lightshow to test my stuff.. easy for comparing. When I did this PR I only tested in Sublime Text with a different theme of course. For my newest code, I'm manually updating the tmlanguage file to test in vscode, but this is much simpler.

I'll take a look at the regressions you mentioned when I get home, but I have a fully refactored language file that I have been working on. Would rather get that perfect than use time on these (if they are complicated), so might end up closing this PR entirely and create a new one for the newest version.

[vors]

I think it's worth at least merge it in a branch in the origin for future references and comparison.

[daviwil]

Hey @gravejester, hate to have to ask you to do this, but do you mind closing this PR and creating a new one with the same changes? Apparently the CLA automation tool doesn't pick up existing PRs that were created before it gets added to a repo. Once you do that you'll be able to get the CLA signed so that I can merge the changes.

[gravejester]

@daviwil No problem. Do you want me to create a new PR with the same commits, or should I just go straight for the new refactored file?

[daviwil]

Whichever you prefer is fine with me!

[gravejester]

Clossing this - created new PR