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

[JavaScript] Refactor statements. #1469

Merged
merged 6 commits into from
Mar 27, 2018

Conversation

Thom1729
Copy link
Collaborator

This PR does for statements what #1009 did for expressions. Fortunately, this is a much, much simpler change.

This change:

  • Allows for the removal of some code that worked slightly differently for statements and expressions. This was especially noticeable in the case of import/export statements.
  • Properly treats semicolons as part of the statements they terminate. An immediately observable consequence is that the semicolon ending an import/export statement will be scoped meta.import|export.
  • Scopes the semicolon of an empty statement punctuation.terminator.statement.empty. (These semicolons are usually unwanted.) This will make it much easier to write sensible tests for statements that do not require a semicolon.
  • Fixes the of/in keyword in for loops, which was apparently broken. There are some corner cases that aren't handled perfectly, and in fact distinguishing the two types of for loops perfectly is impossible in Sublime (like distinguishing arrow functions from parenthesized expressions). The new code for this should do the right thing in reasonable cases and fail gracefully elsewhere.
  • Untangles some of the conditional logic for constructs like switch and do-while.
  • Renames some scopes that have drifted from their original purpose.

In addition, it fixes a parsing problem with commas in exports that was visible in JS Custom. The core syntax scopes the rare JavaScript comma operator as punctuation, so the problem was not observable in the core syntax. However, this fix lets us implement #831 if we want to.

As a result of this change, we do have a lot of anonymous contexts providing meta scopes:

    - match: switch{{identifier_break}}
      scope: keyword.control.switch.js
      set:
        - - meta_scope: meta.switch.js
          - include: immediately-pop
        - switch-block
        - parenthesized-expression

These contexts are not reusable, and in my opinion they're easier to understand "inline" rather than factored out. Opinions may vary on this.

@Thom1729
Copy link
Collaborator Author

@wbond FYI, this does fix a fairly visible bug with for ... in loops, so it would be nice to get it into the next dev build if possible. I may have buried the lede in the original comment. The bug was probably introduced by one of the changes implementing destructured bindings.

@wbond
Copy link
Member

wbond commented Mar 23, 2018

I am going to do another pass through the packages before the next dev build, so I should be able to get this in

@wbond
Copy link
Member

wbond commented Mar 27, 2018

I'm definitely not keen on the - - nested structures. For someone who reads YAML pretty regularly, it isn't that big a deal, but it gets harder to determine what indentation level different parts are.

@wbond wbond merged commit 917d9e7 into sublimehq:master Mar 27, 2018
@Thom1729 Thom1729 deleted the javascript-refactor-statements branch March 27, 2018 13:00
@Thom1729
Copy link
Collaborator Author

@wbond I've refactored the meta contexts in #1479 to avoid the nested lists.

deathaxe pushed a commit to deathaxe/sublime-packages that referenced this pull request Jun 9, 2019
…tatements

[JavaScript] Refactor statements.
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.

2 participants