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

Strikeout deprecated CompletionItems #78092

Merged
merged 12 commits into from
Aug 21, 2019

Conversation

kamranayub
Copy link
Contributor

Closes #50972

Changes

  • Add CompletionItem.deprecated
  • Add deprecated flag to SuggestDataDto
  • Add inline styling to suggest widget

@kamranayub
Copy link
Contributor Author

@jrieken I wasn't sure if the way I was applying styles was recommended so definitely looking for feedback there (and whether or not to externalize the CSS class name)

@jrieken jrieken self-assigned this Jul 29, 2019
@jrieken jrieken added this to the August 2019 milestone Jul 29, 2019
@jrieken jrieken added feature-request Request for new features or functionality suggest IntelliSense, Auto Complete labels Jul 29, 2019
@KamasamaK
Copy link

Note that adding a dedicated deprecated property would conflict with @jrieken's proposal at
#23927 (comment)

@kamranayub
Copy link
Contributor Author

@KamasamaK Confused though, because I thought it was already defined in the LSP:

https://microsoft.github.io/language-server-protocol/specification#textDocument_completion

Won't this change match that interface defined there? I assumed the interface is a 1:1 mapping.

@jrieken
Copy link
Member

jrieken commented Aug 6, 2019

Won't this change match that interface defined there? I assumed the interface is a 1:1 mapping.

There is no 1:1 mapping, tho often a conceptual mapping between the extension API and the LSP. @KamasamaK raises a valid point here, not sure how to proceed. Adding a single boolean is obviously the easiest change but we don't wanna do that all the time, e.g. for private, static, readonly, etc, etc. So maybe some upfront design work is needed. Also checking what @dbaeumer's plans for LSP are?

@kamranayub
Copy link
Contributor Author

Without a lot of background/context for this, my uneducated view is that private, static, readonly would be access modifiers while deprecated seems like a separate concern, like an annotation/tag/status.

@jrieken
Copy link
Member

jrieken commented Aug 7, 2019

, my uneducated view is that private, static, readonly would be access modifiers while deprecated seems like a separate concern

Well, we don't wanna be to academic here. Sure 'public', 'private', 'protected', 'friend' etc are access modifiers but there is more, like 'static' or 'final' or Java's multithread modifiers like 'volatile' and 'transient'. I think there is no absolute truth and the API never tried to correctly represent actual language constructs but to be a common denominator of a UI model the reflects those things. So, one can think of a bit mask of such flags that add some extra rendering to a symbol, like a static method, or a private, deprecated constructor

@dbaeumer
Copy link
Member

dbaeumer commented Aug 7, 2019

I would support adding a modifier instead of a separate deprecated field. There is no rule for a 1:1 mapping. And there are requests in LSP to support more than deprecated which would lead to a modifier as well.

@kamranayub
Copy link
Contributor Author

Once I have a hot second I can explore switching to a modifier. 👍

@jrieken
Copy link
Member

jrieken commented Aug 21, 2019

I will go ahead here and merge things and do more the remaining changes on master. Thanks @kamranayub

@jrieken jrieken merged commit 4447834 into microsoft:master Aug 21, 2019
@kamranayub
Copy link
Contributor Author

kamranayub commented Aug 21, 2019 via email

@jrieken
Copy link
Member

jrieken commented Aug 21, 2019

No - thank you!

@kamranayub kamranayub deleted the joh/completionsDeprecated branch August 21, 2019 17:55
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality suggest IntelliSense, Auto Complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strikeout CompletionItems (and Symbols?) that are deprecated
4 participants