[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.

[Microsoft Contribution License Agreements (msftclas)]

All CLA requirements met.

[Jake Bailey (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.

[Jake Bailey (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 <notifications@github.com> 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> .

[Jake Bailey (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

[Jake Bailey (jakebailey)]

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

[Jake Bailey (jakebailey)]

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

[astenman]

Thanks!