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

Fix scan failures on nested compiler directives #262

Merged
merged 3 commits into from
Jun 27, 2024
Merged

Conversation

Cirras
Copy link
Collaborator

@Cirras Cirras commented Jun 19, 2024

This PR:

  • Promotes Token.TokenType to public visibility (incidental QA)
  • Improves directive name parsing in cases where the name is not followed by whitespace
    • For example, the following is a valid IF directive: {$ifǣ <> 123}
  • Fixes scan failures around nested compiler directives

Note that this does not add proper support or evaluation for nested compiler directives.
That would be quite challenging to accomplish without first doing #261.

Fixes #260.

Cirras added 2 commits June 19, 2024 11:49
This type was exposed through public methods, so it shouldn't be
package-private.
@Cirras Cirras requested a review from zaneduffield June 19, 2024 01:53
Copy link
Collaborator

@zaneduffield zaneduffield left a comment

Choose a reason for hiding this comment

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

Looking pretty good overall. I've left a couple of questions and nit-picky suggestions.

@Cirras Cirras force-pushed the recursive_directives branch from 6875948 to e7e4f26 Compare June 26, 2024 10:28
Copy link
Collaborator

@zaneduffield zaneduffield left a comment

Choose a reason for hiding this comment

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

Looks like you're handling all those pesky edge cases now. Everything looks good, although I think you duplicated a test case in one of the resources.

Compiler directives (and comments) can appear nested within the constant
expressions of `{$if}` and `{$elseif}` directives.
@Cirras Cirras force-pushed the recursive_directives branch from e7e4f26 to 434e3e2 Compare June 27, 2024 01:11
@Cirras Cirras requested a review from zaneduffield June 27, 2024 01:12
Copy link
Collaborator

@zaneduffield zaneduffield left a comment

Choose a reason for hiding this comment

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

Looks good!

@Cirras Cirras merged commit 58a870f into master Jun 27, 2024
2 checks passed
@Cirras Cirras deleted the recursive_directives branch June 27, 2024 02:08
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.

Expression lexer failing to compile file in recursive directives
3 participants