Initial support added for textDocument/documentHighlight

[astenman]

Change-Id: Ib13d86bc96a3702b0e0d79b27b7791898388e104

Since I am using this language server in emacs, it would be nice to have support for textDocument/documentHighlight.

The added functionality only looks for related items in the current file.

[msftclas]

All CLA requirements met.

[jakebailey]

Thanks for sending it, what you have so far looks good. The other "sources" we have (completion, hover, references, etc), have tests written, so it'd be appreciated it you could add those to keep things tested.

[astenman]

Good point. I have added a test file with two simple tests.

[jakebailey]

This is great. Just a few style comments. A preview of this in action:

Note how it picks the correct x. Previously, VS Code would just find any x and say "this is x".

[astenman]

I am using the fact (I think it is a fact though) that the first element of the References list can be interpreted as the actual definition and thus be tagged as "write". Den ons 13 nov. 2019 21:08Jake Bailey <[email protected]> skrev:

…

***@***.**** commented on this pull request. ------------------------------ In src/LanguageServer/Impl/Sources/DocumentHighlightSource.cs <#1767 (comment)> : > +using Microsoft.Python.Core.Text; +using Microsoft.Python.LanguageServer.Documents; +using Microsoft.Python.LanguageServer.Protocol; + +namespace Microsoft.Python.LanguageServer.Sources { + internal sealed class DocumentHighlightSource { + private const int DocumentHighlightAnalysisTimeout = 10000; + private readonly IServiceContainer _services; + + public DocumentHighlightSource(IServiceContainer services) { + _services = services; + } + + public async Task<DocumentHighlight[]> DocumentHighlightAsync(Uri uri, SourceLocation location, CancellationToken cancellationToken = default) { + if (uri != null) { + var analysis = await Document.GetAnalysisAsync(uri, _services, DocumentHighlightAnalysisTimeout, cancellationToken); @astenman <https://github.com/astenman> Perhaps I'm confused, but doesn't your PR include that information? Or are you using Read/Write a different way? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1767?email_source=notifications&email_token=AKNTBQVZ6P6HCAAAVWG33YLQTRNE5A5CNFSM4JKMW5R2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCLO4FNY#discussion_r345976953>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKNTBQTQDJYPQPKRHLFYEXDQTRNE5ANCNFSM4JKMW5RQ> .

[jakebailey]

Ah, I understand. This PR looks good to me, though I'm going to want to hold off on merging it immediately (extension release soon, so we're trying to keep things to bugfixes to limit risky changes).

Old

[jakebailey]

Sorry for the delay in reviewing and merging this. LGTM.

[jakebailey]

Will merge once I have done a little bit of testing.

[astenman]

Thanks!