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 scanner and keyword #229

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

aspeddro
Copy link
Collaborator

@aspeddro aspeddro commented Jun 9, 2023

Close #228

The end of and keyword must have a space

(source_file [0, 0] - [4, 0]
  (let_declaration [0, 0] - [0, 20]
    (let_binding [0, 4] - [0, 20]
      pattern: (value_identifier [0, 4] - [0, 8])
      body: (array [0, 11] - [0, 20]
        (number [0, 12] - [0, 13])
        (number [0, 15] - [0, 16])
        (number [0, 18] - [0, 19]))))
  (expression_statement [2, 0] - [2, 15]
    (pipe_expression [2, 0] - [2, 15]
      (subscript_expression [2, 0] - [2, 7]
        object: (value_identifier [2, 0] - [2, 4])
        index: (number [2, 5] - [2, 6]))
      (value_identifier_path [2, 9] - [2, 15]
        (module_identifier [2, 9] - [2, 11])
        (value_identifier [2, 12] - [2, 15]))))
  (expression_statement [3, 0] - [3, 15]
    (pipe_expression [3, 0] - [3, 15]
      (subscript_expression [3, 0] - [3, 7]
        object: (value_identifier [3, 0] - [3, 4])
        index: (number [3, 5] - [3, 6]))
      (value_identifier_path [3, 9] - [3, 15]
        (module_identifier [3, 9] - [3, 11])
        (value_identifier [3, 12] - [3, 15])))))

// Ignore new lines before `and` keyword (recursive definition)
in_multiline_statement = true;
advance(lexer);
if (is_whitespace(lexer->lookahead)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should rather check against valid identifier characters (a-z, A-Z, apostrophe, $) rather than whitespace, because and (trailing space) is more or less the same as and( (trailing lparen) or and/*comment*/ (a comment right after). What do you think?

Also, you consume an extra character I’m not sure it won’t break anything. For example, the case with a comment and/*comment*/

Would you add some tests?

Also, you consume

Copy link
Collaborator Author

@aspeddro aspeddro Jun 10, 2023

Choose a reason for hiding this comment

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

I think it should rather check against valid identifier characters (a-z, A-Z, apostrophe, $) rather than whitespace, because and (trailing space) is more or less the same as and( (trailing lparen) or and/comment/ (a comment right after). What do you think?

I think we should scan for white space as the scanner is after a line break \n, so if an and followed by a white space is found then in_multiline_statement is true.

Note that and is a keyword, so and followed by any character is a syntax error.

@aspeddro aspeddro marked this pull request as draft March 21, 2024 02:03
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.

Error: and keyword scanner
2 participants