refactor: Do not use .metals for checking the metals version

[tanishiking]

partially tackle #1143

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 the extension.

[dos65]

@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

[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.

Are you talking about the coursier mirror settings?

metals-vscode/src/mirrors.ts

Lines 28 to 31 in bfc4de1

	const mirrorContents = [
	"metals.from=https://repo1.maven.org/maven2",
	`metals.to=${mirrorString}`,
	].join("\n");

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.

[tgodzik]

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.

[kpodsiad]

If I understand correctly, depending on where user set checkForUpdates switch then version will be stored in that scope (global vs workspace), I'm right?
Here, because it's a simple boolean there are no such cases like with server version itself (where empty string in workspace is treated as a valid value and global setting is discarded)

[tanishiking]

If I understand correctly, depending on where user set checkForUpdates switch then version will be stored in that scope (global vs workspace), I'm right?

Yes, you're right!

Here, because it's a simple boolean there are no such cases like with server version itself

What do you mean?
This is the same behavior with the previous one.

Do you mean, we should check for updating both Global and Workspace settings, even if suggetUpgradeSetting is true only in Global or Workspace?
In that case, I agree, but I think we can work on that in another issue / PR #1071

[kpodsiad]

@tanishiking I honestly don't remember what I was thinking and what I meant 😅 (even after reading my comment)

[tanishiking]

haha, no worries! sorry for my delayed reply

[tanishiking]

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 👍
(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...)

[tanishiking]

Also added unit test (while refactoring the project structure) for needCheckForUpdate tanishiking#82
I'll open a PR against this repository after this PR has merged.

[tgodzik]

(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 😅

[dos65]

Looks good!

[tanishiking]

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!