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

AST breaks with ( in declaration section #64

Closed
aMOPel opened this issue Oct 28, 2023 · 4 comments · Fixed by #71
Closed

AST breaks with ( in declaration section #64

aMOPel opened this issue Oct 28, 2023 · 4 comments · Fixed by #71
Labels
bug Something isn't working

Comments

@aMOPel
Copy link
Contributor

aMOPel commented Oct 28, 2023

let 
  a: int = 
let_section [93, 0] - [94, 10]
  "let" [93, 0] - [93, 3]
  variable_declaration [94, 2] - [94, 8]
    symbol_declaration_list [94, 2] - [94, 3]
      symbol_declaration [94, 2] - [94, 3]
        name: identifier [94, 2] - [94, 3]
    type: ":" [94, 3] - [94, 4]
    type: type_expression [94, 5] - [94, 8]
      identifier [94, 5] - [94, 8]
  ERROR [94, 9] - [94, 10]
    "=" [94, 9] - [94, 10]

when typing ( it breaks the (let_section)

let 
  a: int = (
"let" [93, 0] - [93, 3]
symbol_declaration_list [94, 2] - [94, 3]
  symbol_declaration [94, 2] - [94, 3]
    name: identifier [94, 2] - [94, 3]
":" [94, 3] - [94, 4]
type_expression [94, 5] - [94, 8]
  identifier [94, 5] - [94, 8]
"=" [94, 9] - [94, 10]
"(" [94, 11] - [94, 12]
@alaviss alaviss added the bug Something isn't working label Nov 1, 2023
@alaviss
Copy link
Owner

alaviss commented Dec 11, 2023

I have a fix for this by restructuring how statements might end, but the parser size grew over 100MiB, which is over GitHub's limit.

I think we will have to consider working with downstream users about downloading parser via GitHub Releases or similar mechanism. It is clear that parsing Nim correctly requires a very big parser.

@aMOPel
Copy link
Contributor Author

aMOPel commented Dec 11, 2023

That would mean rolling out your own download/build tool and not using the service of nvim-treesitter.

Plus, moving our queries out of nvim-treesitter would greatly reduce discoverability.

You could make it part of your nim.nvim plugin I guess. Still wouldn't be where I would expect to find the treesitter parser/queries.

@aMOPel
Copy link
Contributor Author

aMOPel commented Dec 11, 2023

Is it not possible to add LFS to the repo and still have everything work as before? Dunno much about LFS.

@alaviss
Copy link
Owner

alaviss commented Dec 11, 2023

Given that tree-sitter 1.0 have "not shipping parser.c within repo" as a goal, I think sooner or later all downstreams will have to move.

I'd prefer to have nvim-treesitter carry the parser still, so going that path does require their cooperation.

Git LFS still requires the user to have Git-LFS installed, and not to mention that there is a GitHub-enforced quota for using them, where usage in fork also counts against the main repo...

I'll still work on optimizing the parser size to avoid needing a custom download pipeline, but this is not easy.

alaviss added a commit that referenced this issue Dec 11, 2023
scanner: allow end on EOF and use column for flag invalidation

Allowing layout_end on EOF allows the parser to close and isolate grammar portions where layout_end is typically not allowed (ie. in parentheses), which enabled better error recovery.

The empty termination hack does not seem to contribute anymore with this change and was removed.

With this change, we also remove the use of synchronize node for fixing after newline. Instead it is cleared based on column position at the start of the lexer. This should reduce errors from node reuse, but is more or less a hunch since it is hard to test incremental parsing.

Fixes #64.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants