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

feat(lsp): add workspace folder initialization #912

Merged
merged 5 commits into from
Sep 23, 2024

Conversation

tris203
Copy link
Contributor

@tris203 tris203 commented Sep 9, 2024

Fixes #911

  • Added logic to walk through workspace folders and initialize templ files.
  • Logs warnings when source maps are not found in cache.
  • Parses templates and sets source map cache contents.
  • Handles errors during file reading, template parsing, and generation.

works on #911

- Added logic to walk through workspace folders and initialize templ files.
- Logs warnings when source maps are not found in cache.
- Parses templates and sets source map cache contents.
- Handles errors during file reading, template parsing, and generation.
@tris203
Copy link
Contributor Author

tris203 commented Sep 9, 2024

This PR preloads the templ files into sourcemaps

This works, but the issue still persists, will need to look closer as to why

@tris203
Copy link
Contributor Author

tris203 commented Sep 10, 2024

Okay, so before the file is open, when we forward the request to gopls, it returns a result that is off by 1 (in the line only, character is fine)

This throws our mapping off. I have no idea why that would be the case at the moment

- Added a global variable `preLoadURIs` to store URIs to be preloaded.
- Modified `Initialize` method to append URIs to `preLoadURIs`.
- Updated `Initialized` method to preload URIs by calling `DidOpen`.
- Ensured `DidOpenTextDocumentParams` are created and appended correctly.
@tris203
Copy link
Contributor Author

tris203 commented Sep 10, 2024

Okay, this works.

We store the loaded URIs and their contents and then fire them into didOpen once the servers are all initialized.

I don't know if I like this implementation, but it works.

If anyone has a moment, can they give a pre-review and any thoughts on how it can be improved?

Moved the preLoadURIs variable from a package-level variable to a field
within the Server struct
@tris203 tris203 marked this pull request as ready for review September 21, 2024 18:45
@tris203 tris203 requested a review from joerdav September 21, 2024 18:45
@joerdav
Copy link
Collaborator

joerdav commented Sep 23, 2024

I'm going to take some time to test this out today in various projects to ensure it works as expected.

I also wonder if we can lock this bug away with an lsp test: https://github.com/a-h/templ/blob/60631f964f1c347f3ef5ff34caab3e01b968855c/cmd/templ/lspcmd/lsp_test.go

@joerdav
Copy link
Collaborator

joerdav commented Sep 23, 2024

Happy to add this changes if you don't have time, let me know! And thankyou for the contribution and deep dive into this bug!

- Add new test cases in `TestReferences` to handle references from remote
  files that have not been opened.
- Introduce `remoteChild.templ` and `remoteParent.templ` files for testing
  remote inclusions.
- Update existing test cases to include filename in the test struct.
@tris203
Copy link
Contributor Author

tris203 commented Sep 23, 2024

I'm going to take some time to test this out today in various projects to ensure it works as expected.

I also wonder if we can lock this bug away with an lsp test: 60631f9/cmd/templ/lspcmd/lsp_test.go

I have added a test (and fixtures to provide for this) have a look and see if you think this is good enough

@joerdav
Copy link
Collaborator

joerdav commented Sep 23, 2024

Tested this on my own projects and works as expected. Thanks!!

@joerdav joerdav merged commit fb6a5c2 into a-h:main Sep 23, 2024
4 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.

Neovim/Template goto definition returns error: index out of range
2 participants