Skip to content

Support completion of unimported types.#1896

Merged
JoeRobich merged 5 commits intoOmniSharp:masterfrom
333fred:import-completion
Aug 19, 2020
Merged

Support completion of unimported types.#1896
JoeRobich merged 5 commits intoOmniSharp:masterfrom
333fred:import-completion

Conversation

@333fred
Copy link
Copy Markdown
Contributor

@333fred 333fred commented Aug 18, 2020

This needs to call a few internal overloads for completion, instead of the public versions, in order to determine whether any completions can be expanded (ie, imported) and to get the full change for these items, including the import. The public APIs do not provide this information currently.

Todo:

  • More testing!
  • Make import completion optional. In large solutions it will almost certainly be expensive, we should provide an option to turn it off.

Preview of the change in vscode (watch the line numbers!):
import-completion

Fixes #1482

This needs to call a few internal overloads for completion, instead of the public versions, in order to determine whether any completions can be expanded (ie, imported) and to get the full change for these items, including the import. The public APIs do not provide this information currently.
@333fred 333fred marked this pull request as draft August 18, 2020 07:09
Comment thread src/OmniSharp.Abstractions/Models/v1/Completion/CompletionItem.cs Outdated
Comment thread src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs Outdated
* Added global option for controlling whether import completion is turned on, off by default.
* Additional testing.
@333fred 333fred marked this pull request as ready for review August 19, 2020 04:58
@filipw
Copy link
Copy Markdown
Member

filipw commented Aug 19, 2020

it looks like the tests are a bit unstable for example this failed on Linux

    OmniSharp.Roslyn.CSharp.Tests.CompletionFacts.ImportCompletionResolvesOnSubsequentQueries(filename: "dummy.cs") [FAIL]
      System.OperationCanceledException : The operation was canceled.
      Stack Trace:
          at System.Threading.CancellationToken.ThrowOperationCanceledException () [0x00010] in <a17fa1457c5d44f2885ac746c1764ea5>:0 
          at System.Threading.CancellationToken.ThrowIfCancellationRequested () [0x00008] in <a17fa1457c5d44f2885ac746c1764ea5>:0 
          at OmniSharp.Roslyn.CSharp.Tests.CompletionFacts+<>c__DisplayClass9_0.<ImportCompletionResolvesOnSubsequentQueries>b__1 () [0x000d2] in <720a8e872b3940aca18dd68ab266aa89>:0 
          at OmniSharp.Roslyn.CSharp.Tests.CompletionFacts.ImportCompletionResolvesOnSubsequentQueries (System.String filename) [0x001ee] in <720a8e872b3940aca18dd68ab266aa89>:0 

but passed on a retry. Maybe some sort of a thread exhaustion, these agents are not too powerful

Copy link
Copy Markdown
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit: :shipit: :shipit:

@333fred
Copy link
Copy Markdown
Contributor Author

333fred commented Aug 19, 2020

I'll up the cancelation limit. 100 ms is very stable locally, but I was a bit curious how they'd do on CI.

@333fred
Copy link
Copy Markdown
Contributor Author

333fred commented Aug 19, 2020

I'll up the cancelation limit. 100 ms is very stable locally, but I was a bit curious how they'd do on CI.

@filipw upped the timeout to a second/test.

@filipw
Copy link
Copy Markdown
Member

filipw commented Aug 19, 2020

thanks this is good to go IMHO! 🍻

@JoeRobich
Copy link
Copy Markdown
Member

Thanks Fred!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Import completion for types

4 participants