dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.04k stars 4.03k forks source link

Improve completion support for obsolete members #26488

Open sharwell opened 6 years ago

sharwell commented 6 years ago

This is a design issue related to completion. Implementation issue(s) will be created according to the approved design, once available.

Original issues:

Options for improving the use of these members include, but is not limited to the following:

xCyborg commented 5 years ago

I hope someone is on this.

gundermanc commented 4 years ago

To answer @CyrusNajmabadi's question from the previous thread, yes, VS's completion is able to present strikethrough text encoded using the unicode facilities for strikethrough, however, typing the regular text didn't match the strikethrough version from the completion list when I tried it.

image

My guess is that you could probably get the desired behavior using the right combination of CompletionItem properties:

The same thing looks possible in language server protocol by providing both a label and sort text.

@AmadeusW is the feature owner for completion and might know more.

AFAIK we don't currently support dimming of items in the in-proc completion API. LSP protocol does have a property indicating deprecation status though, so, it's possible to add support for that in both places.

CyrusNajmabadi commented 4 years ago

When you are bolding items, you look at the display text though right? so we would lose bolding here, right?

Note: my preference would be to not have the language set strikethrough, but instead expose a boolean stating if this is deprecated/obsolete. Then, the platform would apply the styling and could do the right thing for all clients in a consistent fashion. Would that make sense for you @gundermanc ?

airbreather commented 4 years ago

In my ideal world, it works like this (n.b.: I haven't seriously looked at how other languages / IDEs handle this):

In table form: ObsoleteAttribute.IsError Severity Action
true * filter out
false Error ~strikethrough~
false Warning gray text
false Info gray text
false Hidden gray text (or, perhaps, do nothing)
false None (i.e., suppressed) do nothing

Perhaps another alternative would be to decorate the list item the same way that it would be decorated in the text editor after completion? e.g., if the completion would result in an error, then give it a red squiggly underline in the list too.

Regardless, as an expert user, I would also appreciate being able to (opt-in) filter out all obsolete items from completion lists.

gundermanc commented 4 years ago

Would that make sense for you @gundermanc ?

@AmadeusW, as completion owner can you chime in?

PathogenDavid commented 4 years ago
  • Reasoning: the compiler will always emit an error unless you change the member itself, so I see no reason to have the IDE help you add errors to your code.

Personally I'd want to see it there anyway because I think it would cause confusion for people who knew about the now-obsolete API and the obsolete message would hopefully point me to the replacement API.

jasonmalinowski commented 3 years ago

Design meeting notes: we agreed we should go ahead with strikethrough, since VS code implemented that. If we get feedback we could also add a filter, but we seem to get be getting a small number of users using filters today.

@genlu will chat with the editor team next to see if this is already supported.

gjrfytn commented 3 years ago

@jasonmalinowski will it also be possible to show obsolete members in the end of the suggestion list? An option to hide them completely sounds great for me too (without the need to hide them every time suggestions show up, I mean some kind of persistent filter).

jasonmalinowski commented 3 years ago

Possibly! I'll let @genlu speak to the details. (I was just the note taker.)

AmadeusW commented 3 years ago

Looks like LSP spec offers CompletionItemTag.Deprecated - are we going to go with this for the LSP implementation? I can add a property to the VS CompletionItem

CyrusNajmabadi commented 3 years ago

are we going to go with this for the LSP implementation

Yup.

I can add a property to the VS CompletionItem

Awesome! Let us know when it's available. Thanks!

genlu commented 3 years ago

An option to hide them completely sounds great for me too (without the need to hide them every time suggestions show up, I mean some kind of persistent filter).

I think this make sense, and making it the default would be my preference..

DamianEdwards commented 3 years ago

Apparently VS Code has a feature like this for symbols in the editor, not just in completion. Would be great to have that in the VS editor too:

image

Original tweet that brought this to my attention: https://twitter.com/JustinNoelDev/status/1337007045011234817