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

[WIP] Rewrite syntax definition in YAML #165

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

michaelblyons
Copy link
Collaborator

@michaelblyons michaelblyons commented Feb 1, 2020

Goals

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

New improvements

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

Regression log

  • I stole the test assertions from PowerShell/EditorSyntax to watch for regressions, but they are not exhaustive.
  • 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 michaelblyons self-assigned this Feb 1, 2020
@theaquamarine
Copy link
Contributor

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.

@deathaxe
Copy link
Member

deathaxe commented Feb 3, 2020

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.

@michaelblyons
Copy link
Collaborator Author

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. 😄

@rwols
Copy link

rwols commented Mar 22, 2020

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)

@michaelblyons
Copy link
Collaborator Author

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.

@rwols
Copy link

rwols commented Jul 19, 2020

ping @vors

@FichteFoll
Copy link
Member

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. 😉

@michaelblyons
Copy link
Collaborator Author

michaelblyons commented Mar 28, 2021

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

  • Tag a 3.1.0 release on master.
  • Switch PCC to tags. Switch PowerShell package to tagged releases wbond/package_control_channel#8236
    • One release tag for ST versions that do not support the new features; and one for those that do.
  • Tag this PR as 4.0.0-alpha.01
  • Add a no-change release of 3.1.1 to ST3 with a message inviting activation of pre-releases.
  • Deal with early adopters whose cheese was moved.
  • 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.

Copy link
Member

@FichteFoll FichteFoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only reviewed the latest changeset, since I am now subscribed to the issue, so don't consider this to be a full review.

PowerShell.sublime-syntax Show resolved Hide resolved
michaelblyons and others added 6 commits April 16, 2021 19:52
Use meta.interpolation and punctuation.section.interpolation scopes for
$(...) subexpressions within strings and clear the string scope, to
enable ST's brackets matching.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants