Skip to content

Allow for [Completion|Symbol]Kind modifiers #23927

Closed
Closed
Allow for [Completion|Symbol]Kind modifiers#23927
Assignees
jrieken
Labels
apiapi-finalizationfeature-requestRequest for new features or functionalityverifiedVerification succeeded
Milestone
September 2019

Description

@jrieken
Johannes Rieken (jrieken)
opened on Apr 5, 2017

extracted from #2628

In completions and for symbols we allow to provide a kind which is mostly use to derive an icon. For instance vscode.CompletionItemKind.Method. This issue is about allowing to modify these kinds to expressing that something is a "private member" or an "abstract protected method"

The idea is that we don't add more and more kinds, e.g no CompletionItemKind.PrivateMethod but a second type, a kind-modifier, that works in combination with a kind.

Potential candidates are: public, protected, internal, package-private, friend, private, static, final, const, abstract, volatile, transient, ... A "n languages have this"-rule should be applied when picking candidates.

Activity

  1. Johannes Rieken (jrieken)
    added
    api
    feature-requestRequest for new features or functionality
    on Apr 5, 2017
  2. Johannes Rieken (jrieken)
    self-assigned this
    on Apr 5, 2017

  3. jrieken commented on May 9, 2018

    @jrieken
    Johannes Rieken (jrieken)
    on May 9, 2018
    ContributorAuthor

    cc Kai Maetzel (@kieferrm)

  4. jrieken commented on May 9, 2018

    @jrieken
    Johannes Rieken (jrieken)
    on May 9, 2018
    ContributorAuthor

    More modifier candidates are: deprecated, experimental

  5. sean-mcmanus commented on Nov 26, 2018

    @sean-mcmanus
    Sean McManus (sean-mcmanus)
    on Nov 26, 2018
    Contributor

    For C/C++, we would like "declaration", "private", and "protected".

  6. Johannes Rieken (jrieken)
    added
    api-proposal
    on Aug 21, 2019

  7. jrieken commented on Aug 21, 2019

    @jrieken
    Johannes Rieken (jrieken)
    on Aug 21, 2019
    ContributorAuthor

    Different options

    1. Instead of adding a new enum, add special members to the existing CompletionItemKind enum, e.g ModifierReadonly, ModifierStatic etc. Then use everything as bit mask, e.g. kind: CompletionItemKind.Function | CompletionItemKind.ModifierDeprecated | ModifierReadonly`
    2. Add a new enum, e.g. CompletionItemKindModifier, and redefine the kind-property to be kind?: CompletionItemKind | { base: CompletionItemKind, modifier?: CompletionItemKindModifier }. That ensures a modifier never occurs without base kind.

    I am in favour of option 2 but I am unsure if we should have two XYZKindModifier enums CompletionItemKindModifier and SymbolKindModifier or one. I don't like the former but I feel it's the correct way of doing it...

    Adding Matt Bierner (@mjbvz) for feedback

  8. mjbvz commented on Aug 21, 2019

    @mjbvz
    Matt Bierner (mjbvz)
    on Aug 21, 2019
    Contributor

    Yes I prefer option 2 as well.

    Here's what some other projects do:

    From an api point of view, I like the TypeScript approach that uses an array of modifiers:

    kind?: CompletionItemKind | { base: CompletionItemKind, modifiers?: readonly CompletionItemKindModifier[] }

    That way an extension could mark a completion as readonly and static or as optional and external for example. However it would make the ux design tricky as there would be even more combinations kinds and kind modifiers

  9. sean-mcmanus commented on Aug 21, 2019

    @sean-mcmanus
    Sean McManus (sean-mcmanus)
    on Aug 21, 2019
    Contributor

    The result our C/C++ extension wants is just the ability to have our completion/symbol icons match VS:
    Completion has a max of 1 modifier:
    image

    Symbols has a max of 2 modifiers:
    image

    Looking at C#, they have an additional "internal" icon modifier too (the "protected internal" and "private protected" cases are not given unique icons). Only 1 modifier max.

    From a UI design/implementation perspective, it seems like the max icon modifiers would be around 3, with the base icon image rendered in the top left (bigger than the modifiers), and potential modifiers rendered on the bottom left, bottom right, and top right. Unless modifiers could change rendering of the base icon in other ways, such as color modulation or a transparent overlay over the entire base icon (e.g. "strikeout").

    Should the API reflect the intended UI effect?

    An AccessModifierKind that renders in the bottom right and a DefinitionModifierKind that renders in the bottom left would work for us. An OtherModifierKind could go in the top right -- not sure what languages would use that though. Maybe an OverlayModifierKind for a strikeout effect and a ColorModifierKind for making icons partially transparent?

    Maybe an API that just gave more control over the icon rendering to extensions would make more people happy instead of having the API be so abstract with just "kinds" that are detached from the actual results.

  10. jrieken commented on Aug 22, 2019

    @jrieken
    Johannes Rieken (jrieken)
    on Aug 22, 2019
    ContributorAuthor

    I like the TypeScript approach that uses an array of modifiers:

    Hm, I was thinking about a bitmask but an array might be JavaScript-like. This also compares to DiagnosticsTag-arrays.

    An AccessModifierKind that renders in the bottom right and a DefinitionModifierKind that renders in the bottom left would work for us. An OtherModifierKind could go in the top right

    Sean McManus (@sean-mcmanus) I don't think we wanna be the fine grained. Generally, we have to walk the fine line of finding a good enough abstraction that describes the our UX and that still fits most programming languages.

  11. jrieken commented on Aug 22, 2019

    @jrieken
    Johannes Rieken (jrieken)
    on Aug 22, 2019
    ContributorAuthor

    Current proposal (due to TypeScript gymnastics the name is kind2 in the implementation)

    	export enum CompletionItemKindModifier {
		Deprecated = 1
	}

	export interface CompletionItem {

		/**
		 *
		 */
		kind?: CompletionItemKind | { base: CompletionItemKind, modifier: ReadonlyArray<CompletionItemKindModifier> };
	}
  12. Johannes Rieken (jrieken)
    added 2 commits that reference this issue on Aug 22, 2019

    kindModifier is a Set, #23927

    ee2b890

    tweak UI for deprecated completions, update API proposal #23927

    1848b16

  13. jrieken commented on Aug 22, 2019

    @jrieken
    Johannes Rieken (jrieken)
    on Aug 22, 2019
    ContributorAuthor

    Actually, instead of "modifier" we could use "tags". That term is less overloaded when it comes to programming lingo (and it also matches DiagnosticsTag). So, have CompletionItemKindTag and SymbolItemKindTag

    Matt Bierner (@mjbvz) Preferences? Likes?

  14. 5 remaining items

  15. octref commented on Aug 23, 2019

    @octref
    Pine (octref)
    on Aug 23, 2019
    Contributor

    Prefer the alternative, kind.kind would be weird. The tags name is a bit unspecific, but we have Diagnostic#tags as well so we might as well stay consistent with it.

    Good question... I have choosen the current approach so that we don't have tags without (base) kinds. E.g an item could only be tagged as readonly... Internally the API is two field, but also because kind isn't optional. It might be OK as we have a default kind but it's also a little weird...

    I feel even if you can't makeup your mind to provide a kind you should be able to mark an item as deprecated.

  16. jrieken commented on Aug 26, 2019

    @jrieken
    Johannes Rieken (jrieken)
    on Aug 26, 2019
    ContributorAuthor

    Went with the kind approach and I actually quite like it. I thinks its flexible and expressive enough. These are all the proposed changes to cover suggestion, outline, and workspace symbols

    	export enum SymbolTag {
		Deprecated = 1
	}

	export interface SymbolInformation {
		/**
		 *
		 */
		tags?: ReadonlyArray<SymbolTag>;
	}

	export interface DocumentSymbol {
		/**
		 *
		 */
		tags?: ReadonlyArray<SymbolTag>;
	}

	export enum CompletionItemTag {
		Deprecated = 1
	}

	export interface CompletionItem {

		/**
		 *
		 */
		tags?: ReadonlyArray<CompletionItemTag>;
	}
  17. Johannes Rieken (jrieken)
    modified the milestones: August 2019, September 2019 on Aug 26, 2019
  18. Johannes Rieken (jrieken)
    added
    api-finalization
    and removed
    api-proposal
    on Aug 26, 2019

  19. jrieken commented on Aug 26, 2019

    @jrieken
    Johannes Rieken (jrieken)
    on Aug 26, 2019
    ContributorAuthor

    Moving to September for finalisation

  20. Johannes Rieken (jrieken)
    added a commit that references this issue on Sep 4, 2019

    move SymbolTag and CompletionItemTag apis to stable, #23927

    458ac96
  21. Johannes Rieken (jrieken)
    closed this as completedon Sep 30, 2019
  22. Johannes Rieken (jrieken)
    added
    verification-neededVerification of issue is requested
    on Sep 30, 2019

  23. jrieken commented on Sep 30, 2019

    @jrieken
    Johannes Rieken (jrieken)
    on Sep 30, 2019
    ContributorAuthor

    Verifier: use the vscode-dts-utility to use the latest version of vscode.d.ts and/or vscode.proposed.d.ts

  24. Martin Aeschlimann (aeschli)
    added
    verifiedVerification succeeded
    and removed
    verification-neededVerification of issue is requested
    on Oct 3, 2019
  25. vscodebot
    locked and limited conversation to collaborators on Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

apiapi-finalizationfeature-requestRequest for new features or functionalityverifiedVerification succeeded

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions