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

Handle embedded-content #437

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Handle embedded-content #437

merged 1 commit into from
Oct 16, 2024

Conversation

msaraiva
Copy link
Collaborator

@msaraiva msaraiva commented Oct 16, 2024

Related to #278.

Currently, vscode-elixir-ls handles all template languages without parsing them, which brings a problem that all services, including code completion, hover, go to definition, etc. don't work as expected. Ideally, each template-specific extension should handle their own template and then forward to other services when required, for instance, when inside <style>, <script> or elixir expressions inside { } or <% %>.

This PR adds the "embedded-content" scheme to the list of documentSelector for all languages handled by this extension. This allows any other extension to properly forward any Elixir-related command to be handled by ElixirLS only when actually necessary.

Ideally, this extension (or the next official one) should only handle Elixir language. This way it can work consistently, without having to parse each individual template language. All other templates that have their own syntax should be responsible for handling their own LS requests, even if they, eventually, have to forward the request back to this extension.

A couple of examples of inconsistent code completions, listing Elixir-related suggestions where it shouldn't:

Inside <style>

image

Inside <script>

image

Inside HTML markup

image

By properly dealing with embedded-content, other extensions can handle their own templates, detect other embedded languages and then forward to the proper service. The service is handle by whatever extension the user has installed that is registered to handle the content type.

Here's an example using the Surface extension, which avoids bringing all the suggestions from ElixirLS, even when it makes no sense:

Listing only applicable components and HTML tags

image

Forwarding to Elixir service when inside expressions

image

Forwarding to CSS/Tailwind service when inside <style>

image

The vscode-elixir-ls should do the same when working with embedded templates using ~H or ~F. But that should be addressed on a separate PR.

// one for compiler diagnostics. Template languages that compile down to Elixir AST
// and embed other languages (e.g. HTML, CSS, JS and Elixir itself), should be moved
// here for proper language service forwarding via "embedded-content".
const templateLanguageIds = ["surface"];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"html-eex" and "phoenix-heex" should eventually be moved here.

@msaraiva msaraiva requested a review from lukaszsamson October 16, 2024 16:00
@lukaszsamson
Copy link
Collaborator

Nice @msaraiva. One question: have you checked if ElixirLS handles embedded-content schema URIs correctly? I believe at least some of the providers should work as it essentially is similar to untitled schema

@msaraiva
Copy link
Collaborator Author

Hey @lukaszsamson!

Thanks for reviewing it.

As far as I could test, they all work just fine with embedded-content. I have only tried completions, hover and definitions, so far, but I assume it will work nice with the others. The vscode in those screenshots, for instance, were all using ElixirLS. I believe we could even use untitled if we wanted but maybe it's better to keep the scheme explicit, specially because anyone trying to integrate with our extension following the official Request Forwarding tutorial, would probably use embedded-content as specified there.

@msaraiva msaraiva merged commit ff387ff into master Oct 16, 2024
2 checks passed
@msaraiva msaraiva deleted the handle-embedded-content branch October 16, 2024 21:19
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.

2 participants