-
Notifications
You must be signed in to change notification settings - Fork 338
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
On some errors metals spawning multiply Open Doctor questions dialogs and stealing focus from user. #6988
Comments
forgot to mention that even when that Open Doctor dialog main Metals functionality working as expected (auto competition works) |
I think that should already be fixed in #6960 You should also be able to change the client configuration the same way in the current version |
just tested it (without any extra client configuration) via 'quick-publish-local' and that doesn't change behavior in any way, 'Open Doctor' still appears and still spawning multiply requests. |
Ach, right! I only had a quick look at the issue and didn't realize this is a different things altogether. |
We need to fix one issue upstream in Bloop and provide a way of disabling the pop up for users. |
Wait, It seems that this should already only log by default and not show message request. #6960 did basically that. Maybe bspStatusBarState or statusBarState option is set to ShowMessage ? Otherwise we never default to it. |
How is Metals configured in your case? I can't find any other case that this would fail. |
I have very simple eglot config: (use-package scala-mode
:interpreter
("scala" . scala-mode))
(use-package eglot
:hook (
(scala-mode . eglot-ensure))) is unlikely that bspStatusBarState or statusBarStat configured on eglot side because it's does't has any configuration specific to metals (except https://github.com/joaotavora/eglot/blob/db91d58374627a195b731a61bead9b4f84a7e4bc/eglot.el#L269). |
I later could try launch metals process with '-Dmetals.status-bar=off' just to be sure. |
Maybe the issue is that there is actual support for metals/status -> https://github.com/emacs-lsp/lsp-metals/blob/master/lsp-metals.el#L830 And it's kind of working as expected? In VS Code the command is not prompted to the user. |
And it looks to be on by deault https://github.com/emacs-lsp/lsp-metals/blob/master/lsp-metals.el#L875 |
As for the error itself, |
Last I saw it was in scalacenter/bloop#1234 |
@tgodzik
This issue only about first case.
|
I will try to test it with -Dmetals.status-bar=off later today. About scalaJs example project I actually have plans to do that for different purpose but not sure on time-frame about that. |
If you find a reproduction for that bridge issue do let me know, I might be able to work in it in Bloop. I will also keep my eyes peeled for that error. |
That's unexpected, could you create |
I will also add some debug for the future that can be activated with |
It seems it must be defined somewhere else, since the defaults do not show messages for Metals. There needs to be some configuration somewhere. What is https://github.com/joaotavora/eglot/blob/master/eglot.el#L270 these references? What is |
I will test it with 'debug' and trace at evening.
thast possible names of program to run. it just looking for program in PATH and if no any then it asking user to provide command (is how I able to launch it with -Dmetals.status-bar=off). Im testing it by putting metals file to ~/.local/bin/ (without any other version installed) |
BTW all these messages come from BSP status. You can make it log info instead of showing the dialog by adding a server property |
@kasiaMarek as I mention in discussion above I already tested it with -Dmetals.bsp-status-bar=log-message and anyway got Open Doctor dialog with that. |
O sorry, my bad for not reading the whole thread. |
@tgodzik
Also I noticed weird error in lsp.trace.json:
so something trying to call Just in case that process looks like this:
|
That seems another bug, worth raising a separate issue. From your trace it indeed seems like bsp status is not enabled at all, so this should not be shown. I will try to repro this myself and see if maybe there is some weirdness. |
OK, so my reproduction worked finally #7052 |
great to hear, thanks. I will try it tomorrow. |
I have no idea how I didn't figure out earlier. Sorry it took so long! |
just in case, I tested it and looks like 'Open Doctor' message no more appears. Anyway, as result I got few more problems but those I able to resolve by myself, I just wanna share some details, just in case. [Trace - 09:51:40 PM] Sending request 'window/showMessageRequest - (6)'
Params: {
"actions": [
{
"title": "Turn off old server"
},
{
"title": "I\u0027ll update manually"
},
{
"title": "Don\u0027t show again"
}
],
"type": 2,
"message": "You have Bloop 2.0.5 installed and Metals requires at least Bloop 2.0.6.\nIf you installed bloop via a system package manager (brew, aur, scoop), please upgrade manually.\nIf not, select \"Turn off old server\". A newer server will be started automatically afterwards.\n\n"
} It's looks like some UX issue because it asking for user confirmation each time but that error can't be resolved in current editing session so I believe better approach there somehow to exit and allow user to continue his session with text editor without LSP. Reason why I got that was because some wrong Jdk path values were cached inside project .bloop folder. It looks like it same issue as this #4053. I believe it something that causing lot of troubles no only with NixOS, I definitely recalling some cases which were caused by it on the MacOS as well. Part of this problem that is very unclear for user why Metals trying to use some specific JDK or Bloop version. So maybe one way to improve it is to provide information not only which version currently used but also extract reason why specific version selected by metals or bloop (probably in error messages and in doctor). |
We will be removing that most likely since most people don't understand what is going on, so it's not really useful.
We should be picking up the same version as metals now. we no longer rely on global config. And Metals should be run with whichever version the boostrapped version is supposed to use, so most likely java on PATH |
Describe the bug
I seen this behavior multiply times in different version but would like to provide some details on recent one.
Usually it happens with editing project build definition (in my case build.sc, but I seen same happen with build.sbt too).
On file save the 'Open Doctor' dialog appears and then editor focus moving to it. But issue that it's not only stealing focus but also that it's creating about 5 dialogs at same time and user required to click on each one to close it. Also, after those dialogs will appears on next file safe again (no matter how much time since previous save).
So basically with that almost not possible to edit any build files. I seen that also happening with usual code but that definitely more rare cases, seems build file editing triggering that almost always.
@dos65 tried to check issue little bit, he provided this patch dos65@db5e731
and with metals version compiled from that branch following messages found in lsp-client log:
Also following error presented in metals.log
Env
Editor
Emacs 29.4 (with doom-emacs)
Lsp client
eglot
OS
Linux (NixOS 24.05)
jdk
openjdk version "17.0.9" 2023-10-17
OpenJDK Runtime Environment Temurin-17.0.9+9 (build 17.0.9+9)
OpenJDK 64-Bit Server VM Temurin-17.0.9+9 (build 17.0.9+9, mixed mode, sharing)
build tool
mill 0.12.1
(with scala project which included ScalaJS usage)
Expected behavior
editor never steal focus from user (except new project opening case)
Operating system
Linux
Editor/Extension
Emacs (other)
Version of Metals
v1.4.1, v1.4.2
Extra context or search terms
doctor
The text was updated successfully, but these errors were encountered: