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

Improve completion support for obsolete members #26488

Open
sharwell opened this issue Apr 29, 2018 · 13 comments
Open

Improve completion support for obsolete members #26488

sharwell opened this issue Apr 29, 2018 · 13 comments

Comments

@sharwell
Copy link
Member

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:

  • Formatting, e.g. strike-through, gray text, etc.
  • Custom glyph(s)
  • Modified sort order
  • Filtering to exclude the items altogether
@xCyborg
Copy link

xCyborg commented Mar 29, 2019

I hope someone is on this.

@gundermanc
Copy link
Member

gundermanc commented Aug 27, 2020

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:

  • Display text should be strikethrough
  • Sort text and filter text and insert text should be non-strikethrough so we can find the item on TYPECHAR and insert the right text.

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
Copy link
Member

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
Copy link

airbreather commented Oct 8, 2020

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

  • If ObsoleteAttribute.IsError is true, then the member should be filtered out.
    • 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.
  • Else if the completion item would necessarily cause a diagnostic whose severity is Error, then it should be in the list, but with strikethrough (perhaps put it to the end of the list to reduce clutter on the "real" members).
    • Reasoning: the compiler would emit an error, but there tend to be auto-fixes in these situations, so I can't justify hiding them outright. Strikethrough feels like the next-strongest clue that the member should not be used.
  • Else if the completion item would necessarily cause a diagnostic whose severity is anything other than Error, then I think gray text makes sense here.
    • Reasoning: grey text makes it clear that this member is not in the same league as everything else, but I perceive it as being less forceful than strikethrough.
    • Perhaps we could skip gray text for "Hidden" too..
  • Else (the completion item would not necessarily cause any diagnostics), let's leave it alone.
    • Reasoning: if the situation isn't even bad enough to give it any markings after completion, then I interpret it to mean that it's not bad enough to do anything special before completion either.

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
Copy link
Member

Would that make sense for you @gundermanc ?

@AmadeusW, as completion owner can you chime in?

@PathogenDavid
Copy link
Contributor

  • 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.

@genlu genlu self-assigned this Nov 2, 2020
@jasonmalinowski jasonmalinowski removed the Need Design Review The end user experience design needs to be reviewed and approved. label Nov 2, 2020
@jasonmalinowski
Copy link
Member

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
Copy link

gjrfytn commented Nov 3, 2020

@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
Copy link
Member

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

@AmadeusW
Copy link
Contributor

AmadeusW commented Nov 3, 2020

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
Copy link
Member

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
Copy link
Member

genlu commented Nov 3, 2020

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
Copy link
Member

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

@sharwell sharwell moved this to Complete in IDE: Design review Aug 22, 2023
sharwell added a commit to sharwell/roslyn that referenced this issue Feb 16, 2024
sharwell added a commit to sharwell/roslyn that referenced this issue Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Complete
Development

No branches or pull requests