-
-
Notifications
You must be signed in to change notification settings - Fork 33
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: add workspace initialization command #374
base: main
Are you sure you want to change the base?
feat: add workspace initialization command #374
Conversation
"[javascript]", | ||
"[typescript]", | ||
"[typescriptreact]", | ||
"[javascriptreact]", | ||
"[json]", | ||
"[jsonc]", |
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.
We should add vue
, astro
, svelte
, css
and graphql
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.
added it in as well
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.
@ematipico If this means Biome will take precedence over the built-in extensions for vue/astro/svelte files, I'm not sure that's a great idea until support has progressed further and stabilized. I suspect anyone using these frameworks in earnest will prefer the official extensions at the moment, which means we'll need to document how to remove these lines from settings.json
/.code-workspace
(which is the kind of complexity that tends to intimidate new users).
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.
that's a valid point, I left it generic in the beginning as I wasn't sure if I should add specific languages as well but yeah this is definitely something to think about from the maintainers side of view!
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.
I think @colinhacks makes a good point here. We should only include languages for which we have full formatting support.
This would mean removing vue
, svelte
, and astro
for now.
Thanks for the contrib. How does that behave when in a a multi-root workspace ? |
@nhedger seems to get added to test.code-workspace instead of settings.json in the individual ones, what would be the right behaviour here? |
I think we'd want the config to be added to the |
@nhedger when you click on the .code-workspace it will generate it on the root of that workspace into a separate .vscode folder, and if you click on the workspaces inside it will generate it there, would that be the expected behavior? |
I would expect it to add the config inside the .code-workspace file in the settings section. |
@nhedger I can't get it to output it to the .code-workspace file at all, seems that vscode isn't happy with that, when i scope it to the .code-workspace file it's still output to the .vscode folder and whatever I do it's the same result, the only way to allow it on .code-workspace is to manually parse the file and rewrite it with fs it seems, unless I'm missing something. |
my findings: |
you can easily check if it's a .code-workspace by checking if the args.path ends with that extension and feel free to try it out yourself and see if i've missed something obvious. From my perspective the PR is ready as is, but that feature can be added on top of course! |
Thanks Alem, I love this! I do have a question for the maintainers. Why are the per-language settings necessary in the first place? If Biome is set as the top-level default formatter in a workspace, it seems surprising that is isn't used by default for all file types that Biome is capable of formatting. I'm sure this was an intentional decision, but I'd love to see it revisited, especially as Biome matures (and with a new major of the extension on the horizon). If Biome adds support for additional file types down the line, those will need to be added. This is likely to cause confusion for users who have run the "Biome: Initialize" and assumed Biome will just work moving forward. (On the flip side of this coin, it's probably not a good idea to override the official Svelte/Vue/Astro extensions yet, see my comment in the review interface.) |
It isn't necessary, actually. For example, I use the following setting on my VSCode: {
"editor.defaultFormatter": "biomejs.biome",
"editor.formatOnSave": true,
} These settings work as expected. If they don't, there's probably a bug lurking somewhere (either the extension or the editor). I suppose the docs are outdated, and at that time, we suggested an opt-in approach not to disrupt the user's settings and default formatter. We are happy to revisit them if you think it's more valuable. There's an open PR for the upcoming v3 of the extension if you're interested: biomejs/website#974 |
@ematipico I also believe there is a bug somewhere, we have been extensively using biome in our company and we had to play around with the settings.json a lot to get it working properly with biome. I've added the suggested changes by @colinhacks to the PR |
Hey guys, any updates on this, anything on my side I need to do? Really interested if it will be merged or not! |
Hey @AlemTuzlak, haven't had a chance to test it out yet. I've been taking a break. I'll try to review and test it this week. |
@nhedger no worries, just wanted to confirm I'm not the cause of it. Great to hear, take your time |
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.
Hey, I took some time to go through the PR.
Seems to work well overall!
Biome: Initialize
command
Calling the Biome: Initialize command from the command palette produces an error message. I suspect it's because the code expects a path that isn't provided when calling the command this way.
I think we should either hide the command from the command palette, or make sure that it works when called this way. I haven't looked into it deeply, not sure if/how it's doable.
FYI, I reached for the command through the palette when testing workspaces in which there was no Biome configuration file, nor VS Code settings, so I could not access the context menu.
@nhedger Sorry, was a bit busy, I've figured out how to hide the command from their docs here: Getting it to work properly from the command palette might be a bit tricky as you'd probably need to specify a file location. |
Summary
Added a command that allows you to initalize biome settings in the workspace based on this tweet:
https://x.com/colinhacks/status/1841219884287737886
and this dicussion:
#230
Description
when you right click on either setttings.json in .vscode or biome.json you can click on the biome: initalize workspace command to add all the required fields into settings.json for biome to work properly, also check this tweet for the merging of the config files:
https://x.com/andhaveaniceday/status/1841226098706956625
Feel free to edit and add whatever you deem is needed here
Checklist