Add semicolon as a commit character for object creation and symbol completion by Cosifne · Pull Request #48851 · dotnet/roslyn

Cosifne · 2020-10-22T18:24:55Z

When trying to complete a method or a type in object creation context by a semicolon, append ();
Put the caret between parenthesis based on if the method/constructor takes a parameter.

Partially resolved #12363

Also need to sync with the intellicode team to make some similar changes to PythiaCompletionProvider.

Dotnet-GitSync-Bot · 2020-10-22T18:24:59Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

Cosifne · 2020-10-23T20:26:28Z

Tag @genlu in case you didn't see this PR 😀

src/Features/CSharp/Portable/Completion/CompletionProviders/ObjectCreationCompletionProvider.cs

genlu · 2020-10-23T21:18:51Z

Also need to sync with the intellicode team to make some similar changes to PythiaCompletionProvider.

In addition to that, I think this could be supported by import completion as well. For those items, we either can recover the symbol from item (for extension methods) or have the fully qualified name to find the symbol from compilation (for type).

src/Features/CSharp/Portable/Completion/CompletionProviders/SymbolCompletionProvider.cs

genlu

I have some perf concerns, let's figure out if the calculation can be reduced and delay to commit time (also, it might worth to get some perf traces to confirm it.) And ideally, I'd like to implement this feature in all relevant providers to ensure a consistent user experience, at least for the ones we own if coordinating with intellicode team takes time.

Also, I remember we spent a bunch of time dogfooding the statement completion feature, do we want to do the same for this (either dogfooding private built vsix or via experiment)?

…erties

…ne/roslyn into dev/shech/methodCompletion

…ethodCompletion

src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs

src/Features/CSharp/Portable/Completion/CompletionProviders/SymbolCompletionProvider.cs

src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs

Cosifne · 2020-11-12T18:46:36Z

tag @genlu in case you didn't see this

src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs

genlu · 2020-11-12T19:57:27Z

The code is much easier to understand now, thanks :)

genlu · 2020-11-12T19:58:19Z

Consider add a few tests to CSharpCompletionCommandHandlerTests. Those tests behave like semi-integration tests since they go through async completion layer as well.

...es/CSharpTest/Completion/CompletionProviders/ExtensionMethodImportCompletionProviderTests.cs

...e/Portable/Completion/Providers/ImportCompletionProvider/AbstractImportCompletionProvider.cs

genlu · 2020-11-12T20:22:47Z

LGTM, just some minor comments. Are you planning to add "." support next?

Cosifne · 2020-11-12T20:32:29Z

LGTM, just some minor comments. Are you planning to add "." support next?

@genlu
Planning to do it in a separate PR.
I think it would be almost the same as ";".

src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs

genlu · 2020-11-13T21:39:55Z

src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs

            return isGeneric ? item.AddProperty("IsGeneric", isGeneric.ToString()) : item;
        }

+        public static CompletionItem AddShouldProvideParenthesisCompletion(CompletionItem item)


Add comment that this is used like a flag so value doesn't matter

genlu · 2020-11-13T21:46:39Z

src/Features/Core/Portable/ExternalAccess/Pythia/Api/PythiaCompletionProviderBase.cs

        public static CompletionDescription GetDescription(CompletionItem item)
            => CommonCompletionItem.GetDescription(item);

+        public static bool GetShouldProvideParenthesisCompletion(CompletionItem item)


Not sure if you added this because you saw my (now deleted) comment, but I think this is actually unnecessary, since intelliccode controls both creation of the item and create the change

genlu · 2020-11-13T21:52:31Z

...e/Completion/CompletionProviders/ImportCompletion/ExtensionMethodImportCompletionProvider.cs

+        // However, for an extension method like
+        // static class C { public static int ToInt(this Bar b) => 1; }
+        // it can only be used as like: bar.ToInt();
+        // bar.ToInt is illegal since it can't be assign to delegate.


TIL this is not allowed :)

genlu

Cosifne added 3 commits October 21, 2020 22:35

Modify SymbolCompletionProvider and ObjectCreationProvider

108e3b1

Add unit tests

79a4626

Renaming

7a93ef3

Cosifne requested a review from a team as a code owner October 22, 2020 18:24

Merge branch 'master' into dev/shech/methodCompletion

4a14daa

vatsalyaagrawal assigned genlu Oct 23, 2020

Fix the LSP test

314cc29

runfoapp bot mentioned this pull request Oct 23, 2020

Roslyn failing to clean up listeners #48816

Closed

genlu reviewed Oct 23, 2020

View reviewed changes

src/Features/CSharp/Portable/Completion/CompletionProviders/ObjectCreationCompletionProvider.cs Outdated Show resolved Hide resolved

genlu reviewed Oct 23, 2020

View reviewed changes

src/Features/CSharp/Portable/Completion/CompletionProviders/SymbolCompletionProvider.cs Outdated Show resolved Hide resolved

genlu requested changes Oct 23, 2020

View reviewed changes

Cosifne and others added 4 commits October 26, 2020 16:17

Improve perf

9861e01

Improve perf in ObjectCreationCompletionProvider

559c7b3

Add text change in AbstractExtensionMethodImportProvider

c0bcee7

Merge branch 'master' into dev/shech/perfTestForMethodCompletion

137c759

jinujoseph added the Area-IDE label Oct 27, 2020

runfoapp bot mentioned this pull request Oct 28, 2020

Roslyn test .AsynchronousOperationListenerTests.Cancel #48988

Closed

Cosifne and others added 10 commits October 28, 2020 13:36

Change the code to let all AbstractImportCompletionProviders add prop…

6b88ad6

…erties

Merge branch 'dev/shech/methodCompletion' of https://github.com/Cosif…

e98d5ab

…ne/roslyn into dev/shech/methodCompletion

Handle text change in abstract import completion provider

b0b3bc0

Add options & experiment flag

6bfb71f

Add dot completion

f263f76

Fix test

1e16635

Change title

12069c8

Add test for import type provider

8f5424a

deley the parenthesis location check when get change

c922156

Merge branch 'dev/shech/perfTestForMethodCompletion' into dev/shech/m…

9f6fe83

…ethodCompletion

genlu reviewed Nov 9, 2020

View reviewed changes

src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs Outdated Show resolved Hide resolved

genlu reviewed Nov 10, 2020

View reviewed changes

src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs Outdated Show resolved Hide resolved

genlu reviewed Nov 10, 2020

View reviewed changes

src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs Outdated Show resolved Hide resolved

genlu reviewed Nov 10, 2020

View reviewed changes

src/Features/CSharp/Portable/Completion/CompletionProviders/SymbolCompletionProvider.cs Outdated Show resolved Hide resolved

genlu reviewed Nov 10, 2020

View reviewed changes

src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs Outdated Show resolved Hide resolved

Cosifne added 2 commits November 11, 2020 12:17

Address feedback

d7a3e8e

remove one unnecessary test

1272b5b

Cosifne requested a review from genlu November 11, 2020 20:29

genlu reviewed Nov 12, 2020

View reviewed changes

src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs Outdated Show resolved Hide resolved

genlu reviewed Nov 12, 2020

View reviewed changes

...es/CSharpTest/Completion/CompletionProviders/ExtensionMethodImportCompletionProviderTests.cs Show resolved Hide resolved

genlu reviewed Nov 12, 2020

View reviewed changes

...e/Portable/Completion/Providers/ImportCompletionProvider/AbstractImportCompletionProvider.cs Outdated Show resolved Hide resolved

Cosifne added 2 commits November 12, 2020 22:50

Address nits, add tests

58116d2

Code style fix

9e49363

genlu reviewed Nov 13, 2020

View reviewed changes

src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs Outdated Show resolved Hide resolved

genlu reviewed Nov 13, 2020

View reviewed changes

genlu approved these changes Nov 13, 2020

View reviewed changes

runfoapp bot mentioned this pull request Nov 14, 2020

Compilation errors cause CI to fail #49020

Closed

Clean nits

fb3b530

Cosifne merged commit d701176 into dotnet:master Nov 14, 2020

ghost added this to the Next milestone Nov 14, 2020

allisonchou modified the milestones: Next, 16.9.P2 Nov 24, 2020

genlu mentioned this pull request Jan 5, 2021

Add one more commit character #49863

Merged

Conversation

Cosifne commented Oct 22, 2020

Uh oh!

Dotnet-GitSync-Bot commented Oct 22, 2020

Uh oh!

Cosifne commented Oct 23, 2020

Uh oh!

Uh oh!

genlu commented Oct 23, 2020

Uh oh!

Uh oh!

genlu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Cosifne commented Nov 12, 2020

Uh oh!

Uh oh!

genlu commented Nov 12, 2020

Uh oh!

genlu commented Nov 12, 2020

Uh oh!

Uh oh!

Uh oh!

genlu commented Nov 12, 2020

Uh oh!

Cosifne commented Nov 12, 2020

Uh oh!

Uh oh!

genlu Nov 13, 2020

Choose a reason for hiding this comment

Uh oh!

genlu Nov 13, 2020

Choose a reason for hiding this comment

Uh oh!

genlu Nov 13, 2020

Choose a reason for hiding this comment

Uh oh!

genlu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants