Add special case to object creation completion to not commit 'object' on '{' #4117
Conversation
… on '{'.
When completion preselects 'object', it becomes very cumbersome for a user to type 'new {' to create an anonymous object. Instead, they get 'new object {' inserted into the editor, which is never really desirable since System.Object doesn't have any properties. This tweak changes completion not to commit in the very specific case of typing '{' when the underlying preselected completion item is System.Object.
Fixes issue dotnet#4115.
|
Reviewers: @rchande, @dpoeschl, @Pilchie, @jasonmalinowski. |
| void M1() | ||
| { | ||
| M2(new | ||
| } |
There was a problem hiding this comment.
This doesn't seem right to me. Shouldn't { be in the editor after this?
There was a problem hiding this comment.
If { isn't a commit chat, does the list actually dismiss, or does it stay up with nothing selected?
There was a problem hiding this comment.
The unit test infrastructure is weird here. We're testing the TextChange that gets inserted by the completion item but it doesn't actually type the character into the editor.
There was a problem hiding this comment.
'{' should dismiss the completion list since we're no longer typing a word.
There was a problem hiding this comment.
Ok. As long as you have verified that behavior, I'll 👍
Maybe a comment about the expected result in the test here though.
There was a problem hiding this comment.
Sure thing. I'm not really keen on this test infrastructure. I may take a look at cleaning that up to better state expectations.
There was a problem hiding this comment.
OK, I went ahead and updated the unit test infrastructure.
|
👍 |
…characters Ensure that the VerifyProviderCommit helper correctly inserts commit characters into the editor (or at least a reasonable approximation) to allow unit tests using this helper to better state their expectations.
| // properties, the user never really wants to commit 'new object {' anyway. | ||
| var namedTypeSymbol = (completionItem as SymbolCompletionItem)?.Symbols.FirstOrDefault() as INamedTypeSymbol; | ||
| if (namedTypeSymbol?.SpecialType == SpecialType.System_Object) | ||
| { |
There was a problem hiding this comment.
Should you verify that this item was really preselected (either by checking the preselect bit or the type of the provider)? What if I'm trying to commit "object" with '{' and it wasn't preselected (not that anyone necessarily does this)?
There was a problem hiding this comment.
Because this rule is only for items returned by the ObjectCreationCompletionProvider, all of the items are preselected. Other "objects" that appear in the list will come from different providers (like the symbol or keyword completion providers) and not have this rule. Thus, they continue to commit on '{'.
There was a problem hiding this comment.
Gotcha. I'd forgotten that you had made that change.
|
👍 |
|
jenkins, retest this please. |
Add special case to object creation completion to not commit 'object' on '{'
When completion preselects 'object', it becomes very cumbersome for a user to type 'new {' to create an anonymous object. Instead, they get 'new object {' inserted into the editor, which is never really desirable since System.Object doesn't have any properties. This tweak changes completion not to commit in the very specific case of typing '{' when the underlying preselected completion item is System.Object.
Fixes issue #4115.