-
-
Notifications
You must be signed in to change notification settings - Fork 223
Add default script language completion #1272
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
Conversation
jasonlyu123
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
Can you also document the config to the README?
packages/language-server/src/plugins/typescript/features/CompletionProvider.ts
Show resolved
Hide resolved
| "title": "Svelte: Rename", | ||
| "description": "Enable rename/move Svelte files functionality" | ||
| }, | ||
| "svelte.plugin.svelte.defaultScriptLanguage": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we put this inside completions? Like "svelte.plugin.svelte.completions.defaultScriptLanguage"? @jasonlyu123 what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. It is also being used in the code action. if we want to further organize it. Maybe the name needs to be more generic. Maybe something like suggest or preferences?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, it's also part of the code action. In that case, never mind 😄 My fear is that people would mistake this as something like "if I set this I don't need to write <script lang="ts"> at all" (like the deprecated and discouraged default language of svelte preprocess).
|
Thanks @devmattrick for getting this together. I just wanted to ask before a maintainer here merges: is this something that should instead go in I see this issue #422, would it be more straightforward to use that method instead? Does the VS Code extension pickup your svelte.config.js file? |
|
The config you mention is deprecated and discouraged. Problem is that not all tooling can reliably read the config, and there's also syntax highlighting limitations - as a result you get all kinds if weird behavior. This PR should only be about adding |
|
@dummdidumm got it, thank you! |
|
does not work?! expexted |
|
nope. This PR is for the script tag created when there's an auto-import. <Cto <script lang="ts">
import Component from 'path/to/Component.svelte'
</script>
<Component |
|
"an auto-import" hmm. what about snippets, is that possible by Svelte config? |
This PR fixes #1262 by adding a new configuration option:
svelte.plugin.svelte.defaultScriptLanguage. This option can either benoneorts, which will configure Svelte VSCode to use<script>or<script lang="ts">respectively when generating new script tags on auto import.Let me know if you'd like any changes. Thanks!