Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't show enum completion if text is typed after dot #18577

Merged
merged 2 commits into from
Apr 11, 2017

Conversation

rchande
Copy link
Contributor

@rchande rchande commented Apr 10, 2017

Fixes #18359
Tag @dotnet/roslyn-ide for review

{
static void Main(string[] args)
{
M(E.a$$)
Copy link
Member

Choose a reason for hiding this comment

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

probably good to have a comment saying that the normal symbolcompletionprovider gives us the values here, so we don't need to duplicate it. Also, we probably want a test in symbolcompletionprovidertests to make sure that we do get values from it.

@@ -52,7 +52,9 @@ internal override bool IsInsertionTrigger(SourceText text, int characterPosition
return;
}

var token = tree.FindTokenOnLeftOfPosition(position, cancellationToken);
var token = tree.FindTokenOnLeftOfPosition(position, cancellationToken)
.GetPreviousTokenIfTouchingWord(position);
Copy link
Member

Choose a reason for hiding this comment

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

is this a problem for VB as well?

Copy link
Member

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

👍 But please valiate VB and add the reverse C# test.

@rchande
Copy link
Contributor Author

rchande commented Apr 10, 2017

Thanks @CyrusNajmabadi , VB looks good and I added the comment/test you suggested.

@rchande rchande merged commit f418671 into dotnet:master Apr 11, 2017
@rchande rchande deleted the fix18359 branch April 11, 2017 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants