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

Add label details to completion entry #48429

Merged
merged 8 commits into from Mar 30, 2022
Merged

Add label details to completion entry #48429

merged 8 commits into from Mar 30, 2022

Conversation

gabritto
Copy link
Member

Follow-up to #48168, as per this comment.
This PR adds label details in our completion entry type.
Since not all editors support this (vscode does, I think VS doesn't, or at least the TS extension in VS doesn't yet), there is also a new flag in UserPreferences that should be used by editors to indicate if they support this new property.

I've also changed the way object literal method snippet completions work to use label details, if they are available.
If they're not available, then we emulate it by modifying the name of the completion entry to be the full signature instead of just the name of the method (e.g. name: foo becomes name: foo(x: number): number.

Examples for what it looks like in vscode:
With label details:
image

Without label details:
image

@typescript-bot
Copy link
Collaborator

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @amcasey, @mjbvz, @minestarks for you. Feel free to loop in other consumers/maintainers if necessary

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 25, 2022
@gabritto gabritto changed the base branch from main to gabritto/issue46590 March 25, 2022 20:36
Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Daniel about the name, but otherwise looks good to me. Thanks for taking this up!

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I correct in understanding that we never set description?

src/services/completions.ts Show resolved Hide resolved
@andrewbranch
Copy link
Member

Long term, description should replace sourceDisplay. However, the latter supports display parts, while the former is a plain string. We only ever send over a single display part with kind text, so I don’t think we’re losing anything important. Maybe it would be nice to start using description and transform it into sourceDisplay based on the setting, in the same way name is transformed.

Base automatically changed from gabritto/issue46590 to main March 30, 2022 00:19
@gabritto gabritto marked this pull request as ready for review March 30, 2022 00:52
@gabritto gabritto merged commit f57bdaa into main Mar 30, 2022
@gabritto gabritto deleted the gabritto/entrydetails branch March 30, 2022 19:45
sidharthv96 added a commit to sidharthv96/TypeScript that referenced this pull request Apr 1, 2022
* upstream/main: (473 commits)
  Correct node used for isDefinition calculation (microsoft#48499)
  fix(48405): emit dummy members from a mapped type (microsoft#48481)
  CFA for dependent parameters typed by generic constraints (microsoft#48411)
  No contextual typing from return types for boolean literals (microsoft#48380)
  fix(47733): omit JSDoc comment template suggestion on node with existing JSDoc (microsoft#47748)
  Ensure that we copy empty NodeArrays during transform (microsoft#48490)
  Add a new compiler option `moduleSuffixes` to expand the node module resolver's search algorithm (microsoft#48189)
  feat(27615): Add missing member fix should work for type literals (microsoft#47212)
  Add label details to completion entry (microsoft#48429)
  Enable method signature completion for object literals (microsoft#48168)
  Fix string literal completions when a partially-typed string fixes inference to a type parameter (microsoft#48410)
  fix(48445): show errors on type-only import/export specifiers in JavaScript files (microsoft#48449)
  Fix newline inserted in empty block at end of formatting range (microsoft#48463)
  Prevent looking up symbol for as const from triggering an error (microsoft#48464)
  Revise accessor resolution logic and error reporting (microsoft#48459)
  fix(48166): skip checking module.exports in a truthiness call expression (microsoft#48337)
  LEGO: Merge pull request 48450
  LEGO: Merge pull request 48436
  fix(48031): show circularity error for self referential get accessor annotations (microsoft#48050)
  Revert "Fix contextual discrimination for omitted members (microsoft#43937)" (microsoft#48426)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants