Skip to content

Conversation

@psteinroe
Copy link
Collaborator

@psteinroe psteinroe commented Jan 12, 2025

migrates the typechecker to the new infrastructure.

  • I did not introduce caching yet (despite it being possible) because it's fast enough imo.
  • using run_async requires static lifetimes, so we need to clone all params - me not likey likey but fixing this probably requires a larger refactor. UPDATE: this is improved by running the typechecker in a single run_async :)
  • I think we should break up the pull_diagnostics method and run one part async and another sync. UPDATE: I nerdsniped myself into doing the refactor. dont know if it actually makes a difference but it was a nice learning :)
  • I did not spend much time implementing the adviser visitor trait for the typechecker diagnostics. i think we can keep it like that and modify it when feedback arrives. we could also think about parsing the pg error in specific diagnostic categories, e.g. typecheck/missingColumn etc but that requires a bit of play to find out about the different error types.
  • any diagnostics other than the main message does not appear yet in the lsp output. need to check if its supposed to work.
  • previously I used the custom parsers result to know where exactly the issue is. I switched this to tree sitter now and it works similarly well. At this point I am wondering why I even worked on the parser for half a year when we barely even need it 🥲🥲
Screenshot 2025-01-12 at 22 38 07

note: I found there is still a crash in the change handler during testing. it happens when I add one or more newlines at the end of the file and then remove it again. will take a look this week. :)

Copy link
Collaborator

@juleswritescode juleswritescode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!!

@psteinroe psteinroe merged commit 9b92948 into main Jan 14, 2025
5 checks passed
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.

3 participants