Fix crashing on window/showMessageRequest
#794
Conversation
It's been crashing for me with the `state.next` returning quoted numbers like `"3"` which then unsuccessfully parsed as `toml::Value`.
| Box::new(MessageRequestResponse { | ||
| message_request_id: jsonrpc_core::Id::deserialize(id).unwrap(), | ||
| message_request_id: id, |
There was a problem hiding this comment.
Thanks and sorry about that; we're missing a lot of test coverage.
Fortunately it's easy to test this manually with metals on an empty *.scala file.
I'll push a slightly different fix since this one might not work if the server treats id: "123" differently from id: 123.
Something else I noticed is that I see a glitch when multiple messages are sent; it looks like
LSP: info from server metals: ⚠️ Compiled t_f4dd477a3a (0.12s) 🔔1 t/test.scala 1:4 1 sel - client0@[foo]
LSP: info from server metals: ⚠️ Compiled t_f4dd477a3a (0.12s) 🔔1 t/test.scala 1:4 1 sel - client0@[foo]
but when I hold shift and copy those two lines, the text is actually joined in one line, and the trailing ]
LSP: info from server metals: ⚠️ Compiled t_f4dd477a3a (0.12s) 🔔1 t/test.scala 1:4 1 sel - client0@[fooLSP: info from server metals: ⚠️ Compiled t_f4dd477a3a (0.12s) ⌛🔔1 t/test.scala 1:4 1 sel - client0@[foo
it looks like Kakoune and my terminal disagree on the width of 🔔 or
I'll look into it..
There was a problem hiding this comment.
Something else I noticed is that I see a glitch when multiple messages are sent; it looks like
Hm, yeah, I remember I had the same problem in the past which I fixed by changing this line to statusBarProvider = "log-message" (docs).
However, recently I've changed it back to statusBarProvider = "show-message" and have no issues anymore.
I suspect, it's a bug with an old metals version. Mine is the latest 1.4.0 and I can not reproduce it.
There was a problem hiding this comment.
it's a well-known type of bug: the terminal and Kakoune disagree on the width of ⚠️.
In my case, the terminal (foot, which has a custom implementation of wcwidth() says it's width 2 and Kakoune (which uses the glibc one) thinks it's width 1.
FWIW this is how I tested the glibc version:
#define _XOPEN_SOURCE
#include <locale.h>
#include <stdio.h>
#include <wchar.h>
int main() {
setlocale(LC_ALL, "");
#define TEST(s) wprintf(s " %d\n", wcswidth(s, wcslen(s)))
TEST(L"🔎");
TEST(L"💡");
TEST(L"⌛");
TEST(L"⚠"); // 1
TEST(L"⚠️"); // 1 but should be 2
}Unfortunately I keep getting network timeouts on the glibc bug tracker.
See also this comment for example.
This situation is pretty unfortunate, not sure when it will get fixed. In kakoune-lsp we generally work around this by falling back to ascii chars if the system wcwidth is confirmed to be wrong.
Hm, yeah, I remember I had the same problem in the past which I fixed by changing this line to statusBarProvider = "log-message" (docs).
Nice, that sounds like the thing to do as default, until we can expect this issue to be fixed in the user's libc.
Alternatively we could apply our expected_width_or_fallback trick but that requires a Unicode database (today we just use it for 3 hard-code characters).
There was a problem hiding this comment.
reported glibc bug as https://sourceware.org/bugzilla/show_bug.cgi?id=32322 though I wonder if it's better to use a separate library for that
…idth As reported in [^1], we have configured Scala Metals to use Unicode symbols for window/showMessage which are displayed in the status line. Kakoune does not yet know that emoji are width 2, which causes visual glitches. Unfortunately fixing Kakoune is not completely trivial because they don't use wcswidth() but only wcwidth(); this means that we'd need to adjust the code to lookahead by one codepoint to detect any VARIATION SELECTOR-16. Until we fix this, let's switch back to the defaults, e.g. no emoji. Alternatively, we could use logMessage to move away from the status line into the debug buffer but that has the same problem, it's just less visible. [^1]: #794 (comment)
…idth As reported in [^1], we have configured Scala Metals to use Unicode symbols for window/showMessage which are displayed in the status line. Kakoune does not yet know that emoji are width 2, which causes visual glitches. Unfortunately fixing Kakoune is not completely trivial because they don't use wcswidth() but only wcwidth(); this means that we'd need to adjust the code to lookahead by one codepoint to detect any VARIATION SELECTOR-16. Until we fix this, let's switch back to the defaults, e.g. no emoji. Alternatively, we could use logMessage to move away from the status line into the debug buffer but that has the same problem, it's just less visible. [^1]: #794 (comment)
It's been crashing for me with the
state.nextreturning quoted numbers like"3"which then unsuccessfully parsed astoml::Value.