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

[ActionScript] refactor syntax #3558

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

Conversation

jrappen
Copy link
Contributor

@jrappen jrappen commented Nov 13, 2022

  • update to syntax v2
  • add variables
  • move comments to prototype context
  • add named contexts for strings & comments while making use of meta_scope
  • replace captures: 0: ... with scope: ...
  • add more tests

- update to syntax v2
- add variables
- move comments to prototype context
- add named contexts for strings and comments
@jrappen
Copy link
Contributor Author

jrappen commented Nov 18, 2022

To anybody reviewing, you can check I haven't changed the regex for the variables by reversing my changes:

  • remove whitespace
  • replace (?:...) with the original (...)

@jrappen jrappen changed the title [ActionScript] clean up syntax [ActionScript] refactor syntax Nov 18, 2022
Comment on lines 738 to 739
- match: \b(?:{{keywords}})\b
scope: keyword.control.actionscript
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about a dedicated keywords context to apply more detailed scopes such as keyword.control.conditional or keyword.control.loop ... just to get those inline with other syntaxes?

ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
Comment on lines 883 to 890
- match: |-
(?x:
(\d+\.?\d*|\.\d+)
({{constant_numeric_exponent}})?
(L|l|UL|ul|u|U|F|f)?
)
scope: meta.number.actionscript
captures:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it make sense to try to distinguish floats from integers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess, will take another look.

- match: |-
(?x:
(\d+\.?\d*|\.\d+)
({{constant_numeric_exponent}})?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Exponent is used once only. Is it really worth a variable then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A leftover from halfway through the re-write as I wasn't sure if I'd need it again maybe. Might keep it when distinguishing floats and integers.

ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
@jrappen
Copy link
Contributor Author

jrappen commented Dec 10, 2022

I'll address leftover comments later today.

ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
ActionScript/ActionScript.sublime-syntax Outdated Show resolved Hide resolved
@jrappen jrappen marked this pull request as draft December 10, 2022 12:21
@jrappen
Copy link
Contributor Author

jrappen commented Jan 23, 2023

I still have to do the keywords context and update the numbers stuff.

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