Filter out language servers which fail to spawn

[woojiq]

Return Result<Arc<Client>> instead of Arc<Client> so that we can then filter only successfully started language servers and create an error log otherwise.

Closes #8361

[the-mikedavis]

This is currently nice and minimal but I think we should do a slightly larger refactor here since Registry::get is forcing us to collect twice and it's making the call site in Editor awkward. (Although the Option handling there is also awkward, so I say let's fix that in this patch too.)

Let's make Registry::get return an impl Iterator<Item = (LanguageServerName, Result<Arc<Client>>)>. Then when launching the language servers we can filter out the Errs as you already do in this patch, collect into the HashMap, and return early when language_servers is empty.

That launch_language_servers function is returning Option<()> and bubbling it up to refresh_language_servers but the return value is never used. I think the option is very confusing - I don't really understand the distinction between Some(()) and None - so let's remove those return types on refresh_language_servers and launch_language_servers. Each of the ? operators can be replaced with let Some(..) = .. else { return };s

[woojiq]

Done, thanks for the suggestion to return an iterator instead of collecting twice) The diff is a bit crazy because I moved the if content outside, so it looks like I overwrote everything.
We needed to unify all lifetimes in get so that we can use move in the closure (without move - error that closure will outlive borrowed enable_snippets) and also to return the iterator.

[pascalkuthe]

this looks great, thnaks for working on this! I agree this should land before the release. This is also a nice cleanup.

I found one minor nit, otherwise this LGTM

[pascalkuthe]

nit: instead of the early return I think it would be cleaner to just move the last 3 lines into the Ok match arm

[woojiq]

Done.

Off-topic: Do I need to write comments like "Done" after the suggested changes? Do you get notified about new commits of reviewed code? Iirc, I was notified, so these comments are redundant.

[pascalkuthe]

you don't have to write it, I get GitHub notifications when somebody pushes to a PR. In cases like this where its just a small change missing before merge I would check myself (but it doesn't hurt if you leave the comment either). For larger PRs ts nice if you do leave some comments, a push may not necessarily indicate they are done and I often skip those notifications (since I get a lot of those).