kotlin-community-tools / kotlin-language-server

MIT License
19 stars 4 forks source link

Fix internal errors propagating to stderr #10

Closed AlexandrosAlexiou closed 2 weeks ago

AlexandrosAlexiou commented 2 months ago
owl-from-hogvarts commented 3 weeks ago

LGTM. Except, instead of propagating error we are propagating null value now. What is the end result of the null propagation? I see that it results in nulls being displayed in user interface at least somewhere

owl-from-hogvarts commented 3 weeks ago

@AlexandrosAlexiou Notifying you, that our organisation has changed name as of your suggestion. Please, update remote repo url in your local clone

AlexandrosAlexiou commented 3 weeks ago

LGTM. Except, instead of propagating error we are propagating null value now. What is the end result of the null propagation? I see that it results in nulls being displayed in user interface at least somewhere

Which user interface are you referring to? AFAIK, an exception will only print to stderr if it is not caught, here we catch it we log the error and return null from the function. Please correct me if I am wrong.

owl-from-hogvarts commented 2 weeks ago

Speaking of GUI, I referred to Hower.kt:30. But, after more thoroughly reviewing code I can confirm, that null won't get into UI.

[!NOTE] Under UI/GUI I mean strings which are being sent to LSP-client to be displayed in the Clinet's UI at the end.

owl-from-hogvarts commented 2 weeks ago

Nope, null gets its way to UI at Hover.kt:42.

Instead of returning null, null is being interpolated into a string. I will implement the check for null there and merge your PR.

owl-from-hogvarts commented 2 weeks ago

Thanks for contribution!

AlexandrosAlexiou commented 2 weeks ago

Nope, null gets its way to UI at Hover.kt:42.

Instead of returning null, null is being interpolated into a string. I will implement the check for null there and merge your PR.

Thanks!