metals-vscode icon indicating copy to clipboard operation
metals-vscode copied to clipboard

[VSCode] Metals is generating files on click the extension view, even if I'm not in a Scala project

Open RaniAgus opened this issue 1 year ago • 8 comments

Describe the bug

I opened VSCode in an Angular application and accidentally clicked Metals tab. There are two files that got generated and they shouldn't because I'm not in a Scala project and I din't click the "Start Metals" button.

image

image

Expected behavior

No files should be generated

Operating system

Windows

Editor/Extension

VS Code

Version of Metals

v1.1.0

Extra context or search terms

2023.11.29 09:13:21 INFO  Started: Metals version 1.1.0 in folders 'xxxxxx' for client Visual Studio Code 1.84.2.
2023.11.29 09:13:22 WARN  Build server is not auto-connectable.
2023.11.29 09:13:22 WARN  no build tool detected in workspace 'xxxxxx'. The most common cause for this problem is that the editor was opened in the wrong working directory, for example if you use sbt then the workspace directory should contain build.sbt. 

RaniAgus avatar Nov 29 '23 12:11 RaniAgus

Thanks for reporting! This is the default behaviour of VS Code currently. When the tab is opened, VS Code will start the extension responsible for that tab :/ We don't define it ourselves.

There is even a note about the change https://code.visualstudio.com/api/references/activation-events#onView

tgodzik avatar Nov 29 '23 13:11 tgodzik

We might be able not work around it, but not sure currently how to do it.

tgodzik avatar Nov 29 '23 13:11 tgodzik

This will get automatically resolved if we add single files support https://github.com/scalameta/metals/pull/5531.

kasiaMarek avatar Nov 29 '23 13:11 kasiaMarek

Have you considered using a temporary folder for logging instead of a relative path?

https://github.com/scalameta/metals/blob/06b8ea1945a855dbcc539b840c8cdced600cff31/metals/src/main/scala/scala/meta/internal/metals/Directories.scala#L15-L16

RaniAgus avatar Nov 29 '23 13:11 RaniAgus

There could be 2 solutions regarding the issue if https://github.com/scalameta/metals-vscode/issues/1439#issuecomment-1831925338 didn't work, my 2 cents :) I think writing log files into tmp dir would be ok, but I believe (2) is nicer + Metals still has some usage for .metals (should we use /tmp for those purposes too? e.g. swtich-build-server setting, I don't think so, but we may workaround by store the setting on client-side).

  • (1) Maybe, metals client shouldn't initialize the server if the workspace doesn't seem to Scala workspace
    • metals-vscode will be activated if the workspace contains Scala-related files https://github.com/scalameta/metals-vscode/blob/16e7e65ccd4e41355f046300f713ef8f05c9b2c5/packages/metals-vscode/package.json#L45-L58
    • However, metals-vscode will also be activated by onView as @tgodzik mentioned https://github.com/scalameta/metals-vscode/issues/1439#issuecomment-1831908538
    • We might want to double check the existence of Scala-related files in extension.ts or somewhere, and do not initialize the server if it's not Scala workspace (?)
    • Cons
      • it could be a bit error-prone
  • (2) Maybe we don't need to write logs into .metals/metals.log
    • Metals server setup logger to write logs into metals.log around here https://github.com/scalameta/metals/blob/e6303c3eea2dda655f4edcb065337f6a596ce179/metals/src/main/scala/scala/meta/internal/metals/logging/MetalsLogger.scala#L117-L127
    • on the other hand, Metals server also send those logs to the client via window/logMessage
      • https://github.com/scalameta/metals/blob/e6303c3eea2dda655f4edcb065337f6a596ce179/metals/src/main/scala/scala/meta/internal/metals/logging/MetalsLogger.scala#L91-L96
      • https://github.com/scalameta/metals/blob/e6303c3eea2dda655f4edcb065337f6a596ce179/metals/src/main/scala/scala/meta/internal/metals/logging/LanguageClientLogger.scala#L15-L24
      • That's how we see the metals logs in the Output tab of VSCode.
    • VSCode's command > Developer: Open Extension Logs Folder to open the directory that contains logs, and you'll find output_*****/Metals.log (it seems those logs are rotated every 2 weeks by default?)
    • So, we might not need to save .metals/metals.log
    • Cons
      • I sometimes see the difference between the Output and metals.log, we might need to confirm those two has the same contents
      • It will be a bit harder for users to locate the log files
      • Metals still uses .metals directory for other purposes
      • Some LSP client might not store the log files on disk https://github.com/joaotavora/eglot/issues/322

tanishiking avatar Nov 30 '23 00:11 tanishiking

Could a solution be to check if the current folder contains files like *.sbt, *.scala or *.sc before creating the .metals directory?

anatoliykmetyuk avatar Dec 13 '23 12:12 anatoliykmetyuk

Probably something like that, basically duplicating the activation triggers, which we defined :/

tgodzik avatar Dec 13 '23 14:12 tgodzik

The problem was so severe that if the file he generated was deleted, it would be immediately regenerated, which was devastating

YSMull avatar Mar 15 '24 15:03 YSMull