PowerShell icon indicating copy to clipboard operation
PowerShell copied to clipboard

[WIP] Rewrite syntax definition in YAML

Open michaelblyons opened this issue 5 years ago • 8 comments

Goals

  • [x] Use the more-readable sublime-syntax format for the syntax definition, derived from YAML.
  • [x] Adhere more closely to Sublime Text's scope naming conventions.
  • [x] Make it compatible with ST's new regex engine.

New improvements

  • [x] Static members: [type]::function()
  • [x] Things with member accessors: $foo.bar.baz, $foo.bar(1, 6).baz
  • [x] That thing that makes the rest of the line a string: --%

Regression log

  • [x] I stole the test assertions from PowerShell/EditorSyntax to watch for regressions, but they are not exhaustive.
  • [x] echo `"test`" should have escaped "s. Thanks to @jcberquist for finding this.

Further improvement possible

  • [ ] Make some keywords context-specific:
    • e.g. You have to be in ForEach-Object block to have a $foreach automatic variable.
  • [ ] Not all the tests pass
    • But seriously, why would you write 10.05L? If you want a long, why type a decimal on it?
  • [ ] Better glob wildcard scopes. Right now, they're basically multiplication and the where alias.
  • [ ] Better [type] stuff.
  • [ ] Other stuff? Let me know what you find.

If you're feeling adventurous, try it out and let me know what you think. Please comment with (hopefully sane) test cases that I'm missing.

michaelblyons avatar Feb 01 '20 19:02 michaelblyons

Isn't PowerShell/EditorSyntax's PowerShellSyntax.tmLanguage effectively canonical for anything that's not using an actual parser for highlighting? What would moving away from using that offer, when it's maintained by the PowerShell team and tested by the combined userbase of this package, VSCode, and anything else using it?

It's true that the definition in this package isn't updated often, but at least now bugs can be resolved by manually replacing the tmLanguage file in the package's folder, but moving to a different language definition would mean having to port any changes and maintain a separate set of tests.

theaquamarine avatar Feb 02 '20 17:02 theaquamarine

This is the SublimeText organization here. So I can't see a reason why using the latest/proper syntax-format which supports the full set of features the editor's engine has to offer is a problem.

Anyone is free to fork the package for use in other text editors.

deathaxe avatar Feb 03 '20 19:02 deathaxe

The PowerShell "EditorSyntax" tmLanguage is currently... really bad. You might even notice that some contexts will include another context multiple times.

There is an operation underway wherein a committed contributor has been experimenting with making it significantly better... for VS Code. His PR looks good, but it has been ongoing for over a year, and critically, it does not compile with Sublime Text. It does go further in scope than this PR (and attempts to fully comprehend individual statement state, rather than the minimal context-awareness that I have built upon).

I have a branch wherein I have cleaned up that PR to the point where ST can at least parse it, but

  • It misses important things, probably related to the changes I made to make it run at all.
  • It's not compatible with sregex, Sublime's faster regex engine, and has to fall back to Oniguruma.
  • It's taken me twice as long to tinker with that code so far than it has to make this PR here.

I do not have the chutzpah to ask the PR author to rewrite his yearlong effort without the VS Code secret sauce. 😄

michaelblyons avatar Feb 12 '20 01:02 michaelblyons

Built-in functions are highlighted properly, but user functions lack a variable.function scope. For instance, in this snippet addSchedule and injectScheduler are user-defined functions. I'm not sure how feasible it would be to add this feature. It looks to be a significant effort, because variable assignment can have whitespace in-between the =-sign, making it hard to predict whether the current token is going to be a variable assignment or a function call. On the other hand, variable assignments always start with a $-symbol.

for ($i=1; $i -le 3; $i++) {
    addSchedule
    injectScheduler
    start-process $stPath
    $startTime = get-date
    $timeout = $false
    while (-not (test-path $outFile) -or (get-item $outFile).length -eq 0) {
        write-host -nonewline "."
        if (((get-date) - $startTime).totalseconds -ge 10) {
            write-host
            $timeout = $true
            if ($i -eq 3) {
                throw "Timeout: Sublime Text is not responding."
            }
            break
        }
        start-sleep -seconds 1
    }
    if ($timeout) {
        stop-process -force -processname sublime_text -ea silentlycontinue
        if (test-path $schedule_target) {
            remove-item $schedule_target -force
        }
        continue
    }
    write-host
    break
}

(snippet is from https://github.com/SublimeText/UnitTesting/blob/master/sbin/run_tests.ps1 by the way)

rwols avatar Mar 22 '20 16:03 rwols

Does the = have to be on the same line? It's possible that the Bash stuff you wrote can be reproduced here to include a [ \t]*, but the other considerations involved in fully parsing statements is beyond my current ability. 😉

It might be feasible to write something with ST4-style branches, but in order for me to do it, I think I'd have to try BenjaminSchaaf/sbnf, and it won't be releasable until ST4 is out.

michaelblyons avatar Mar 22 '20 18:03 michaelblyons

ping @vors

rwols avatar Jul 19 '20 11:07 rwols

By modern standards, a .sublime-syntax will always have the edge over a tmLanguage file with regards to readability, capability, and maintainability. As such, since we are within the SublimeText organization, I believe it is in our best interest to provide the best experience for ST possible.

There hasn't been a reply to this PR in over a year by a maintainer, so I will exert my ownership power to add a new maintainer. Guillermo, the initiator of this repo, has been AWOL for quite a while now as well.

@michaelblyons, I am adding you as a collaborator to this repo. I know you have enough experience with handling packages, so I trust you to handle this well. Note that the package still uses branch-based releases, so before merging this, you should adjust the PCC entry to use tags (and adjust the ST build selector).

You may also consider revamping the other contents of this package, such as the random color scheme that has no business here imo and whetever snippets or completions are provided. Just remember to also include an update message for the users of the package explaining the potentially radical changes.

And finally, you should change the readme of https://github.com/PowerShell/EditorSyntax to not include this repo anymore. :wink:

FichteFoll avatar Mar 27 '21 23:03 FichteFoll

Thanks to @FichteFoll for the vote of confidence, and to @rwols for the nudge to get this rolling. Here's the tentative plan:

  • [x] Tag a 3.1.0 release on master.
  • [x] Switch PCC to tags. https://github.com/wbond/package_control_channel/pull/8236
    • One release tag for ST versions that do not support the new features; and one for those that do.
  • [x] Tag this PR as 4.0.0-alpha.01
  • [x] Add a no-change release of 3.1.1 to ST3 with a message inviting activation of pre-releases.
  • [x] Deal with early adopters whose cheese was moved.
  • [x] Wait a month. (Possibly longer if there are major regressions.)
  • [ ] Merge and tag a 4.0.0.
  • [ ] Deal with everyone else whose cheese was moved.

michaelblyons avatar Mar 28 '21 01:03 michaelblyons