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

Review trivia which can't be inferred from their node kind #223

Open
c42f opened this issue Mar 20, 2023 · 4 comments
Open

Review trivia which can't be inferred from their node kind #223

c42f opened this issue Mar 20, 2023 · 4 comments

Comments

@c42f
Copy link
Member

c42f commented Mar 20, 2023

Some tokens becomes trivia after parsing has done its work. For example, parentheses in both f(x) and (x) only appear in the green tree as trivia. For the function call, the presence of parentheses can be inferred from the tree (call f x) because there's only one type of function call syntax. That is, we can reconstruct the trivia which isn't whitespace or comments.

In contrast, this is not the case for (x), until #222 is merged because the green tree didn't represent grouping parentheses (these ended up as trivia on the parent node instead).

In general, it seems that a nice invariant for the green tree to have would be that

Every piece of syntax can be inferred from the kind, flags, and list of nontrivia children, other than whitespace and comments.

Presumably there's cases other than #222 to fix - would be good to review all cases of bump used together with TRIVIA_FLAG.

@c42f
Copy link
Member Author

c42f commented Apr 29, 2023

I looked through parser.jl. General impression is that there's still various cases such as trailing commas etc, but they mostly don't matter.

A borderline case is not being able to distinguish A where B from A where {B} without looking at the trivia. It might be nice to add a flag for that I guess.

@c42f
Copy link
Member Author

c42f commented Apr 29, 2023

I don't know, I'm kind of finding myself uninspired by this issue... it seems only marginally useful to change the remaining cases. If we're going to add a flag for A where B does it also makes sense to add one to distinguish things like f(a,b) from f(a,b,)? There's a bunch of such marginal cases and it might not be practical to cover them all vs just dynamically querying the trivia list...

@c42f c42f closed this as completed Apr 29, 2023
@c42f
Copy link
Member Author

c42f commented Apr 30, 2023

Actually let's fix the where case. Not by adding a flag, but by adding an extra tree node to SyntaxNode

@c42f
Copy link
Member Author

c42f commented May 7, 2023

The where case was fixed in #264. I'm not aware of other cases except for oddities like blocks with empty clauses such as "a ;;; b ;; c" or trailing commas like in (a, b,) which ... idk ... I guess could be useful to have a flag for?

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

No branches or pull requests

1 participant