Only send `completionItem/resolve` if supported

[feltech]

• Oni will hang indefinitely if the language server simply chooses to ignore unsupported requests.
• This can be avoided if the server provides correct capabilities and we actually check them before making the request.

[feltech]

This addresses one of the shortcomings I mentioned in https://github.com/onivim/oni/issues/1016#issuecomment-428380453

[feltech]

There are still tests to write, so this is still WIP at the moment.

[feltech]

Also note to self (and whoever else cares): update wiki page to remove warning about completion failing in cquery/ccls once this is merged.

[codecov[bot]]

Codecov Report

Merging #2624 into master will increase coverage by 0.02%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2624      +/-   ##
==========================================
+ Coverage    45.3%   45.33%   +0.02%     
==========================================
  Files         361      361              
  Lines       14569    14572       +3     
  Branches     1912     1913       +1     
==========================================
+ Hits         6601     6606       +5     
+ Misses       7744     7741       -3     
- Partials      224      225       +1

Impacted Files	Coverage Δ
...er/src/Services/Completion/CompletionsRequestor.ts	32.35% <100%> (+19.44%)	:arrow_up:
browser/src/Services/Notifications/Notification.ts	16.66% <0%> (-8.34%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a4d877b...da3ba9b. Read the comment docs.

[akinsho]

@feltech thanks for the PR 👍 not to make this more complex than your original intent I wonder if you've seen anywhere where we could generalise this for other lsp capabilities as this could come up again for another lsp maybe in the handler that calls this function there could be away to check if a capability is available before actually calling the function that handles the capability?

[feltech]

Re. generalising. The thought did occur to me.

It would seem sensible to at least have a final sanity check before sending a command to the language server (which could potentially hang if the command is not supported). However, based on the example of getting completion details, there isn't any immediately obvious automatic mapping between command and capability (e.g. completionItem/resolve to completionProvider.resolveProvider - they're sort-of similar, but not quite).

So I suspect the intention is to treat each command individually, unfortunately. I haven't looked in detail into the language server protocol, or at any other implementation to see how this problem is handled there. Perhaps there is a clever way to deal with this problem in the general case buried in other implementations.

[feltech]

OK, added tests.

I took the liberty of upgrading sinon (spys, stub, etc library). I wasn't quite sure what to do about yarn.lock, since it seemed to include more than just sinon version changes, so I left that out...

[akinsho]

@feltech all for keeping it simple 👍, re then yarn lock we usually keep it updated under version control so other developers can resolve the right dependencies when they install the way it handles resolution of dependencies means it occasionally involves changes to more than one package so I wouldn't worry too much about that