metals-vscode
metals-vscode copied to clipboard
refactor: Do not use .metals for checking the metals version
This PR replaces $PROJECT_DIR/.metals/versions-meta.json
and $HOME/.metals/versions-meta.json"
to workspaceState
and globalState
respectively.
I remember someone has trouble because Metals creates .metals
in their home directory (on Discord, but I couldn't find the link to the discussion).
Anyway, it's best practice to use context.globalState
(or workspaceState
) for managing the state of extension, and it makes easy to test (in the future) 👍
Let me test locally for a while before merging.
@tanishiking I initially wanted to store it in inside settings but vscode doesn't allow writing into unknown fields. Every setting must be declared in package.json
. There is an option to rename global directory from .metals
to smth else
I initially wanted to store it in inside settings but vscode doesn't allow writing into unknown fields. Every setting must be declared in package.json.
Are you talking about the coursier mirror settings? https://github.com/scalameta/metals-vscode/blob/bfc4de113c688338d6776a1e330d4ddedd742c09/src/mirrors.ts#L28-L31 https://scalameta.org/metals/docs/troubleshooting/proxy/ I don't have a plan to replace it with something else at least in this PR.
I don't have a context why metals-vscode has settings to edit the coursier mirror properties, but I'm wondering why users want to configure it via metals-vscode instead of setting global coursier mirrors properties by themselves.
I don't have a context why metals-vscode has settings to edit the coursier mirror properties, but I'm wondering why users want to configure it via metals-vscode instead of setting global coursier mirrors properties by themselves.
It's just a bit more convenient for the users to have it metals instead of manually searching how to do it.
It's just a bit more convenient for the users to have it metals instead of manually searching how to do it.
Ok, it would be awesome if we could completely remove the usage of ~/.metals
directory for metals-vscode
, but we can work on that in another issue / PR :+1:
(I personally think, vscode settings shouldn't change the global behavior (coursier mirror setting in this case). I think people won't expect a change in the vscode setting create/modify the global coursier setting though...)
Also added unit test (while refactoring the project structure) for needCheckForUpdate
https://github.com/tanishiking/metals-vscode/pull/82
I'll open a PR against this repository after this PR has merged.
(I personally think, vscode settings shouldn't change the global behavior (coursier mirror setting in this case). I think people won't expect a change in the vscode setting create/modify the global coursier setting though...)
I understand what you mean, but it was really complicated for people to find the right place for where to put the file and it is far from obvious. Maybe it would be better to have mirror as a parameter in that case, but I think last time I checked that was not possible. And this was hard to get working right :sweat_smile:
it was really complicated for people to find the right place for where to put the file and it is far from obvious. Maybe it would be better to have mirror as a parameter in that case, but I think last time I checked that was not possible. And this was hard to get working right 😅
I see, thank you for sharing the background!
Maybe we can keep those settings without placing proxy files in ~/.metals
directory, I'll take a look!