Update tagstack

[dimbleby]

Fixes #517, #915.

I don't know that I'm completely convinced by this myself:

• the tag stack maybe doesn't behave quite exactly as expected: when you try to go forward (by :tag) that doesn't work; because the things in the tag stack aren't actually tags so they can't be re-discovered that way
• Not sure either about the inconsistency where we update the stack when we jump to a single definition, but don't when we find multiple definitions and go via the quickfix list or whatever.

I've also done a round of cargo updates, though #892 seems to commit this project to being stuck with an old jsonrpc-core.

Re #892 I'd suggest that if the Sorbet developers think that it's OK to add random fields to a JSON-RPC response, they should make that case at json-rpc core. paritytech/jsonrpc#448 looks to be where the conversation happened. (Else fix their language server not to do that.)

[autozimu]

Thanks for contributing!

I share the same concern that if we turn on the switch, the support would be half complete. User might be getting even more confused by the missing support parts. Kind feel this is a slippery path.

[dimbleby]

Yes, a more integrated approach - I think - should be to set the tagfunc to use the language server. (For Vims where that is supported.)

But that will take a bit more effort...

[teasp00n]

I'm pretty keen to get this landed so was doing some exploratory work around it and it's more complicated than it first appeared.

Tagfunc has two modes of operation, one for normal mode and another for insert mode. Normal mode functionality is analogous to LanguageClient_textDocument_definition() and insert mode is basically LanguageClient_textDocument_completion().

So assuming we implement a LanguageClient_tagfunc() function to proxy the calls to their respective lsp functions then it should just work. But what about direct calls to LanguageClient_textDocument_definition()? And what about LanguageClient_textDocument_references() or LanguageClient_textDocument_documentSymbol()?

To be honest I think it is easier to maintain our own stack of Locations and write functions to manipulate this as needed. Users can just bind that to whatever keybind they want. tagfunc/ tagstack integration might make it more complicated than it needs to be.

Are there other benefits to having an actual tagfunc that I've missed?

[dimbleby]

The main reasons for integrating with the tags function would be to make the whole thing feel natively seamless - jump to definition is really very much like :tag and people naturally have a reasonable hope / expectation that this is how it works. And by using the tag stack you'd get a lot of built-in functionality for free.

Personally, I find that the jump list is mostly satisfactory. So while I was willing to try something simple to integrate with tags - per this PR - I am not very motivated to work on anything more elaborate. Which, I guess, is why this is now just sitting here.

I'd be mildly against this client maintaining its own stack of locations. For me, that would be closer to being the worst of both worlds than the best of both: at least according to my way of working it's not very much better than using the jump list, it's a bunch of extra complexity in the client, and it still doesn't achieve integration with native features. But other users - and the maintainer - may feel differently.

[jez]

Re #892 I'd suggest that if the Sorbet developers think that it's OK to add random fields to a JSON-RPC response, they should make that case at json-rpc core

paritytech/jsonrpc#595

Else fix their language server not to do that.

wip: https://github.com/sorbet/sorbet/compare/jez-undiscriminated-union

[micwoj92]

This branch has conflicts that must be resolved