Parse whitespaces #18
Conversation
inaki-amatria
requested review from
alvrogd and
jgonzac
and removed request for
a team
inaki-amatria
force-pushed
the
feature/ParseWhitespaces
branch
from
a156383 to
59e6ad5
Compare
| // This allows escaping newlines everywhere, although this is only valid in | ||
| // preprocessor statements | ||
| /\s|\\\r?\n/, | ||
| $.whitespace, |
There was a problem hiding this comment.
Do you know how this interferes with the \s above, which also includes whitespace?
There was a problem hiding this comment.
whitespace does not consume all the tokens \s consumes, so the rule is still required
There was a problem hiding this comment.
I mean, could it be some whitespaces in \s that should be handled before with$.whitespace? I suppose not to but, just to know if that can happens.
There was a problem hiding this comment.
I would try replacing\s with [\f\t\n\v\r] and see if something changes
There was a problem hiding this comment.
whitespacedoes not consume all the tokens\sconsumes, so the rule is still required
I'm not sure the kind of ambiguity this suggests... Why not? Why are both required?
inaki-amatria
force-pushed
the
feature/ParseWhitespaces
branch
from
59e6ad5 to
7c934b0
Compare
|
|
||
| preproc_function_def: $ => seq( | ||
| preprocessor('define'), | ||
| field('name', $.identifier), | ||
| field('parameters', $.preproc_params), | ||
| field('value', optional($.preproc_arg)), | ||
| token.immediate(/\r?\n/), | ||
| ), | ||
|
|
||
| preproc_params: $ => seq( | ||
| token.immediate('('), commaSep(choice($.identifier, '...')), ')', | ||
| ), | ||
|
|
||
| preproc_call: $ => seq( |
There was a problem hiding this comment.
More than merge, I would say simply remove, right? I don't see how the merge is happening.
| // This allows escaping newlines everywhere, although this is only valid in | ||
| // preprocessor statements | ||
| /\s|\\\r?\n/, | ||
| $.whitespace, |
There was a problem hiding this comment.
whitespacedoes not consume all the tokens\sconsumes, so the rule is still required
I'm not sure the kind of ambiguity this suggests... Why not? Why are both required?
src/scanner.c
Outdated
| static bool scan_whitespace(Scanner *scanner, TSLexer *lexer, | ||
| bool lex_whitespace) { | ||
| if (!iswblank(lexer->lookahead)) { | ||
| return false; | ||
| } | ||
|
|
||
| while (iswblank(lexer->lookahead)) { | ||
| if (lex_whitespace) { | ||
| advance(lexer); | ||
| continue; | ||
| } | ||
| skip(lexer); | ||
| } | ||
|
|
||
| if (!lex_whitespace) { | ||
| return false; | ||
| } | ||
|
|
||
| lexer->mark_end(lexer); | ||
| lexer->result_symbol = WHITESPACE; | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| static bool scan(Scanner *scanner, TSLexer *lexer, const bool *valid_symbols) { | ||
| // Consume any leading whitespace except newlines | ||
| if (scan_whitespace(scanner, lexer, valid_symbols[WHITESPACE])) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Why is this needed, instead of some simple grammar rule? Seems like something you are not telling here.
inaki-amatria
force-pushed
the
feature/ParseWhitespaces
branch
from
7c934b0 to
e946912
Compare
inaki-amatria
force-pushed
the
feature/ParseWhitespaces
branch
from
e946912 to
202d895
Compare
|
I will close this PR since the approach has changed significantly. We can continue the discussion in a new PR, focusing the review on the updated implementation. |
This PR adds whitespace parsing support to
tree-sitter-fortran. To make this work, I mergedpreproc_defandpreproc_function_def, since keeping them separate caused parsing conflicts. Given that macro support is already limited, losing the distinction between these two nodes should be acceptable for now.