search

gleam-lang / tree-sitter-gleam

🌳 A tree-sitter grammar for the Gleam programming language
Apache License 2.0
66 stars 13 forks source link

Indentation with case statements #57

Closed rawhat closed 1 year ago

rawhat commented 1 year ago

I'll preface this with "I don't really know if there will be a resolution here". Also far from an expert in tree-sitter.

image

This is neovim specifically, but I imagine any indent query will run into the same issue. The grammar defines repeat1 for case_clauses, which I am guessing means 1 or more? That's the node used to determine the beginning of indentation. Before any clauses have been added, there is nothing to match on to signify that we should start indentation.

It's not valid Gleam to have an empty case, so changing that doesn't seem correct?

I guess this is more of a question/discussion issue. This functionality does work in VSCode, but obviously that is completely different. Just noting that it's not particularly unusual behavior.

Is there anything that can be changed in the grammar to handle this? If it's also a specific indent query issue, I can try to look more into that.

This might also end up just being a shortcoming of the grammar and indentation, but was just curious to get some info from more knowledgeable people :)

lpil commented 1 year ago

Does it make sense for the tree sitter grammar to be more relaxed than the compiler one? It could parse successfully even without any clauses. I'm not sure the intent with tree sitter.

rawhat commented 1 year ago

That's a really good question, and I don't know the correct answer here. I can look at a few other languages to see how they handle stuff like this. Elixir has the benefit of using the generic do_block for indentation, so they get it easily for this "case" (oops).

I am curious if that would fix this issue though, since I'm not sure which node we'd use to start indentation even with support for no case clauses.

rawhat commented 1 year ago

image It looks like both Rust and JS at least do permit an empty body. Fairly confident that's not valid Rust, so it seems like there is some validity to handling it that way?

image

Here's what we have in the Gleam parser. I think maybe an empty case_clauses would allow for correct indentation? I think I can try tweaking that and seeing if it works, though I may fail at it haha

J3RN commented 1 year ago

Hello 👋 My policy with tree-sitter-gleam has been to hew as close as possible to the "real" Gleam parser, but am generally unopposed to making it somewhat more permissive. Ultimately the two have different audiences; the real parser facilitates higher-level analyses and code generation whereas tree-sitter-gleam assists tooling.

TL;DR: I think empty cases should be permitted

the-mikedavis commented 1 year ago

Fixed by #59