GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Add label details to completion entry by gabritto · Pull Request #48429 · microsoft/TypeScript

Conversation

Gabriela Araujo Britto (gabritto) commented Mar 25, 2022

Copy link

Copy Markdown

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:

Without label details:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

DanielRosenwasser and mjbvz reacted with heart emoji

All reactions

❤️ 2 reactions

TypeScript Bot (typescript-bot) commented Mar 25, 2022

Copy link

Copy Markdown

Contributor

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 Sheetal Nandi (@sheetalkamat), Andrew Casey (@amcasey), Matt Bierner (@mjbvz), Mine Starks (@minestarks) for you. Feel free to loop in other consumers/maintainers if necessary

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

TypeScript Bot (typescript-bot) assigned Gabriela Araujo Britto (gabritto)

Mar 25, 2022

TypeScript Bot (typescript-bot) added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels

Mar 25, 2022

Gabriela Araujo Britto (gabritto) changed the base branch from main to gabritto/issue46590

March 25, 2022 20:36

Gabriela Araujo Britto (gabritto) requested review from Andrew Casey (amcasey) and Andrew Branch (andrewbranch)

March 25, 2022 20:41

Daniel Rosenwasser (DanielRosenwasser) reviewed

Mar 26, 2022

View reviewed changes

Comment thread

src/services/completions.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Daniel Rosenwasser (DanielRosenwasser) reviewed

Mar 26, 2022

View reviewed changes

Comment thread

src/server/protocol.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Andrew Branch (andrewbranch) approved these changes

Mar 28, 2022

View reviewed changes

Andrew Branch (andrewbranch) left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this 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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Andrew Casey (amcasey) approved these changes

Mar 28, 2022

View reviewed changes

Andrew Casey (amcasey) left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this 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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/services/completions.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Andrew Branch (andrewbranch) commented Mar 28, 2022

Copy link

Copy Markdown

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Gabriela Araujo Britto (gabritto) mentioned this pull request

Mar 29, 2022

Add support for label details in TS completion item microsoft/vscode#146270

Merged

Base automatically changed from gabritto/issue46590 to main

March 30, 2022 00:19

Gabriela Araujo Britto (gabritto) added 6 commits

March 29, 2022 17:42


          add label details to completion entry

1836bdb


          Use label details for obj literal method completions

0c37561


          add label details support flag

e6a8abf


          add label details support to fourslash

4864a3e


          support both label details and non-label details in object literal me…

fe5a809

…thod snippets


          CR fixes

41db01c

Gabriela Araujo Britto (gabritto) force-pushed the gabritto/entrydetails branch from 4c028fd to 41db01c Compare

March 30, 2022 00:52

Gabriela Araujo Britto (gabritto) marked this pull request as ready for review

March 30, 2022 00:52

Gabriela Araujo Britto (gabritto) added 2 commits

March 29, 2022 17:59


          fixes after rebasing

a6299ca


          fix tsserver tests

00f0009

Gabriela Araujo Britto (gabritto) merged commit f57bdaa into main

Mar 30, 2022

Gabriela Araujo Britto (gabritto) deleted the gabritto/entrydetails branch

March 30, 2022 19:45

Sidharth Vinod (sidharthv96) added a commit to sidharthv96/TypeScript that referenced this pull request

Apr 1, 2022


          Merge remote-tracking branch 'upstream/main' into fix/46149

e149cc3

* 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)
  ...

Matt Bierner (mjbvz) mentioned this pull request

Apr 7, 2025

Always Show Return Type #61550

Open

Microsoft (microsoft) locked as resolved and limited conversation to collaborators

Oct 22, 2025

Reviewers

Daniel Rosenwasser (DanielRosenwasser)

DanielRosenwasser left review comments

Andrew Branch (andrewbranch)

andrewbranch approved these changes

+1 more reviewer

Andrew Casey (amcasey)

amcasey approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Gabriela Araujo Britto (gabritto)

Labels

Author: Team For Uncommitted Bug

PR for untriaged, rejected, closed or missing bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.