Skip to content

fix: allow try_statement without catch/finally_clause #41

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

Merged

Conversation

theHamsta
Copy link
Contributor

@theHamsta theHamsta commented Apr 15, 2023

Fixes nvim-treesitter/nvim-treesitter#4632

At the moment, try_statement seems to require either a catch or finally clause. This seems to confuse tree-sitter's error recovery when users start with writing only the try block: tree-sitter will parse such incomplete try blocks without a catch/finally clause, like

    try {
    }
    try {
      foo = 1
    }
    try {
      foo = 1;
    }

with widely changing parsing results as the user types. This causes some challenges to editor features like nvim-treesitter's indentation that rely on tree-sitter recognizing try_statement or block. We might find a way to consider all possible intermediate error states in some way, but I wanted to ask whether it would make more sense to let the parser interpret incomplete try_statement in a more meaningful way to support incremental editing (the try block will always be as a child of ERROR node until the users finishes with catch/finally, tough dart seems to require catch/finally for a valid syntax.

@RobertBrunhage please test! You just need to change the git SHA in nvim-treesitter's lockfile.json to this commit.

theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request Apr 15, 2023
theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request Apr 15, 2023
@RobertBrunhage
Copy link

@theHamsta just gave it a try and it's working. The only scenario that seems to be broke for me was the catch statement if you forget to add the argument for catch(e) before opening the {}.

Example

try {
} catch {} <-- // this isn't registered in `InspectTree` as a catch because you don't pass the argument.

Gave it a try in VSCode just to see how that handles it, and it indents even though you forget adding the (e) for catch (e).

If you feel like it's outside the scope for this, feel free to ignore it, but just wanted to let you know!

@TimWhiting
Copy link
Collaborator

@theHamsta I'm going to merge this, but just want to see if you are planning on addressing the other remark @RobertBrunhage pointed out.

Some major changes are going to have to be made for patterns support etc, and I'm going to try to start tackling them.

@TimWhiting TimWhiting merged commit 19f8579 into UserNobody14:master May 4, 2023
theHamsta added a commit to theHamsta/nvim-treesitter that referenced this pull request May 5, 2023
theHamsta added a commit to nvim-treesitter/nvim-treesitter that referenced this pull request May 5, 2023
xasc pushed a commit to xasc/nvim-treesitter that referenced this pull request May 9, 2023
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.

Dart: indentation incorrect for try {} expanding
3 participants