Skip to content

use "variable" kind for parameter completion#2061

Merged
filipw merged 2 commits intoOmniSharp:masterfrom
filipw:bugfix/icon
Jan 8, 2021
Merged

use "variable" kind for parameter completion#2061
filipw merged 2 commits intoOmniSharp:masterfrom
filipw:bugfix/icon

Conversation

@filipw
Copy link
Copy Markdown
Member

@filipw filipw commented Jan 7, 2021

We used to do it this way until the switch to CompletionService. This PR restores the old mapping of parameter to variable.

Fixes #2060
Fixes dotnet/vscode-csharp#4314

@filipw
Copy link
Copy Markdown
Member Author

filipw commented Jan 7, 2021

cc @333fred

@333fred
Copy link
Copy Markdown
Contributor

333fred commented Jan 7, 2021

Interesting. This is copied directly from roslyn, https://github.com/dotnet/roslyn/blob/master/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs#L57. @dibarbet, is Roslyn intentionally using value for parameters there, or is it an oversight?

@filipw
Copy link
Copy Markdown
Member Author

filipw commented Jan 7, 2021

yes I saw this too and got confused. but then tested in VS directly and saw there that locals and parameters use same glyph. is it remapped elsewhere?

@333fred
Copy link
Copy Markdown
Contributor

333fred commented Jan 7, 2021

yes I saw this too and got confused. but then tested in VS directly and saw there that locals and parameters use same glyph. is it remapped elsewhere?

That map is only used by the LSP, so you won't see it unless you're using codespaces.

@dibarbet
Copy link
Copy Markdown
Contributor

dibarbet commented Jan 7, 2021

@dibarbet, is Roslyn intentionally using value for parameters there, or is it an oversight?

good question, it very well could be an oversight, or VS just uses the same icon for both, @allisonchou do you happen to remember?

@allisonchou
Copy link
Copy Markdown
Contributor

good question, it very well could be an oversight, or VS just uses the same icon for both, @allisonchou do you happen to remember?

I'm not sure. 😦 It may have been an oversight. Looking at the file history, it looks like this mapping has been here since LSP was first tested out in Roslyn.

@333fred
Copy link
Copy Markdown
Contributor

333fred commented Jan 8, 2021

Filed dotnet/roslyn#50324 on Roslyn to investigate whether we're using the wrong value as well.

@filipw filipw merged commit e758077 into OmniSharp:master Jan 8, 2021
@filipw
Copy link
Copy Markdown
Member Author

filipw commented Jan 8, 2021

thanks!

@filipw filipw deleted the bugfix/icon branch January 8, 2021 18:43
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.

The intellisense icon for method parameters maybe wrong. The intellisense icon for method parameters maybe wrong.

6 participants