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

[AppleScript] sregex compatibility #4156

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

michaelblyons
Copy link
Collaborator

See #481

There's lots more that could be done to improve this package, but I've killed the lookbehinds and added a couple of subscopes. Also, there's a test file for the first time.

@jrappen
Copy link
Contributor

jrappen commented Feb 13, 2025

Thanks for working on this.

I still have some left-overs from rainy days from autumn with what I started with while going through this package. Should I send you a zip via Discord?

@michaelblyons michaelblyons marked this pull request as draft February 14, 2025 17:18
@michaelblyons
Copy link
Collaborator Author

Okay, now I'm doing more work on it.

@michaelblyons michaelblyons marked this pull request as ready for review February 14, 2025 19:40
@michaelblyons
Copy link
Collaborator Author

michaelblyons commented Feb 14, 2025

Not a full rebuild from spec and there are definitely some broken pieces, but it's lots better. Has some good building contexts if someone wants to push forward with Apple's crazy blocks.

@jrappen
Copy link
Contributor

jrappen commented Feb 14, 2025

I noticed a few things:

  • Language constants (as an example, there are many others), are they actually \b(?i:false)\b or rather \b(?:[Ff]alse|FALSE)\b? Seemed odd at first glance over.
  • If you match a group of words, sometimes you use (one two), sometimes (one\s+two).
  • You could make use of:
    variables:
      leading_wspace: (?:^\s*)
    as you use it a lot.
  • You can just indent the lines you test a bit, makes writing the assertions on the following lines easier. Like:
      test
    # ^^^^ scope
    
    instead of:
    test
    # <- scope
    #^^^ scope
    

@@ -0,0 +1,635 @@
# SYNTAX TEST "AppleScript.sublime-syntax"
Copy link
Contributor

Choose a reason for hiding this comment

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

# SYNTAX TEST "Packages/AppleScript/AppleScript.sublime-syntax"

@michaelblyons
Copy link
Collaborator Author

michaelblyons commented Feb 15, 2025

  • Language constants (as an example, there are many others), are they actually \b(?i:false)\b or rather \b(?:[Ff]alse|FALSE)\b? Seemed odd at first glance over.

Most of those were inherited. I guess I could check them against the spec, but I did see a line in the spec saying that almost everything is case insensitive.

  • If you match a group of words, sometimes you use (one two), sometimes (one\s+two).

Most of this is inherited, too. The only part I remember with a space is in the raw Unicode part, which isn't fully spec-conforming yet anyway.

  • You could make use of:
    variables:
      leading_wspace: (?:^\s*)

I guess I could, but {{leading_wspace}} is longer than ^\s*.

  • You can just indent the lines you test a bit, makes writing the assertions on the following lines easier. Like:

      test
    # ^^^^ scope
    

    instead of:

    test
    # <- scope
    #^^^ scope
    

Good thought. Will change.

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