Vscode: Consider showing completion item detail if available for all list items

Created on 1 Dec 2017  路  35Comments  路  Source: microsoft/vscode

Take the following completion list.

screenshot from 2017-12-02 09-22-52

Each Client item belongs to a different namespace. If the completion item detail was shown immediately for each item then users could, at a glance, easily identify the relevant item out of a list of similarly labeled items. I suppose the argument against is that this can be done by modifying the label to include this, but then what is the role of the detail property?

api-proposal feature-request suggest ux

Most helpful comment

Current state of UI, with Status Bar turned on:

image

Implementation:

vscode.languages.registerCompletionItemProvider('plaintext', {
  provideCompletionItems() {
    return {
      isIncomplete: false,
      items: [
        {
          label: 'foo',
          label2: {
            name: 'foo',
            signature: '(bar)',
            qualifier: 'baz.foo',
            type: 'MyType'
          },
          documentation: 'my documentation',
          detail: 'my detail'
        },
        {
          label: 'foo2',
          label2: {
            name: 'foo2',
            signature: '(bar)',
            qualifier: 'baz.foo',
            type: 'MyType'
          },
          documentation: 'my documentation',
          detail: 'my detail'
        }
      ]
    }
  }
})

All 35 comments

Furthermore, if an extension author does try and add detail to the label to work around this limitation, then it can end up with incorrect highlighting as shown below.

screenshot from 2018-02-07 20-07-35

Any pointers on how to contribute a fix displaying the "detail" field if it's available? This seems like a low-hanging fruit that would greatly improve the user experience in cases like below

2019-02-13 11 37 35

As an extra data point, the Java LS works around this by including the package in the "label":

image

@gabro , when using the label do you find it can cause incorrect highlighting as in the example above, though?

@bmewburn the highlighting works out ok in my experience if you update the filter text to not include the appended namespace.

fyi - I plan on implementing this for the next (Jan 2020) release and that "label + detail" workarounds will then look bad. Because of that we might start with having this behind a setting

This should also help with Java which often presents a very busy IntelliSense box, where too much is rendered as label. cc @akaroml

Screenshot 2020-01-03 at 12 40 08

Current state:

  • Added CompletionList#isDetailsResolved. When this is set to true, or if completionItemProvider has undefined resolveCompletionItem, we mark an item's detail as resolved.
  • Plan to add a setting, editor.suggest.alwaysRevealInlineDetails to toggle between current behavior and new behavior, which shows all resolved details.

details

Current issues:

  • When a completion item provider can async resolve items but doesn't have isDetailsResolved (as shown above for completions from TS Server), going up/down to reveal inline detail doesn't feel nice.
  • Multiple circle-i icons look weird.
  • We need clearer guidance for details. Currently it says:

    /**
     * A human-readable string with additional information
     * about this item, like type or symbol information.
     */
    detail?: string;
    

    It's unclear whether I should provide a valid code snippet, or some random string.

Questions:

  • Default editor.suggest.alwaysRevealInlineDetails to true or false?
  • Drop duplicated details in the details UI or no?

Screen Shot 2020-01-10 at 11 26 11 AM

I'd say show the 鈩癸笍 icon only for the focused element because that's a click region for the doc-window.

@jrieken The issue is, when item has details that's equal to labels (such as break), we don't render the readMore icon at all, so it looks ugly...

a

Yeah, this is really ugly... I start to like what @alexdima suggested. Instead of changing the rendering of detail we could leave it as is and add a new properly labelDetail which we would render right after the label. Needs some more design work...

Current proposal:

export interface CompletionItemLabel {

    /**
     * The name of this completion item's label.
     */
    name: string;

    /**
     * A description of the completion item which is rendered
     * less prominent.
     */
    // description?: string;

    /**
     * Details of the completion item that is rendered less
     * prominent to the right.
     */
    details?: string;
}

export class CompletionItem {

    /**
     * The label of this completion item. By default
     * this is also the text that is inserted when selecting
     * this completion.
     */
    label: string | CompletionItemLabel;

    /**
     * A human-readable string with additional information
     * about this item, like type or symbol information.
     */
    detail?: string;

    /**
     * Other fields...
     */
}

Other alternatives we didn't like:

  • CompletionList#isDetailsResolved: One more field on CompletionList. Also feels weird marking property in another interface as "having to be provided upfront".
  • CompletionItem#labelDetails: Should be an optional property, but needs to be provided upfront and not resolved later. Still look strange. Also later if we decide to add more fields such as labelDescription etc, this would make CompletionItem very messy.

label: string | CompletionItemLabel is better because:

  • label has to be provided upfront anyway
  • As we enrich the UI, we can easily add more fields to CompletionItemLabel

    # Now
    
    <label>                      <details>
    <label>                      <details>
    <label>                      <details>
    
    # Future, Maybe
    
    <label><description>         <details>
    <label><description>         <details>
    <label><description>         <details>
    
  • This "string | ComplexObject for showing simple / complex UI" concept already exists. For example, showQuickPick:

    export function showQuickPick(items: string[] | Thenable<string[]>, ...) {}
    export function showQuickPick<T extends QuickPickItem>(items: T[] | Thenable<T[]>, ...) {}
    

    image

I like it. It fits my use case in that I would have the symbol short name in CompletionItemLabel.name, the namespace in CompletionItemLabel.details and still able to have extra info like additional text edit imports in CompletionItem.detail.

+1 on the idea of having separate detail fields for both the label and the completion item. With the label detail always showing, we can definitely move some of the information from the label name to the label detail, like this mock:

image

What do you think? @fbricon

export interface CompletionItemLabel {
    // the name of the function or variable
    name: string;
    // the signature, without the return type. is render directly after `name`
    signature?: string; // parameters
    // the fully qualified name, like package name, file path etc
    qualifier?: string;
    // the return-type of a function or type of a property, variable etc
    type?: string;
}

ASCII-art "rendering"

<name><signature>        <qualifier>        <type>

@jrieken Do you want early feedback from TS on the idea and the API? Or should we wait until we have a solid proposal for what they should implement?

Would be great to have TypeScript comment on this. I have seen that they have much of this information after the resolve call but this is information that must be returned from the provide-call. fyi - there are came in https://github.com/microsoft/vscode/issues/88757 which I have re-purposed for that 馃

We are merging below into proposed API:

export interface CompletionItem {

    label: string;

    /**
     * Will be merged into CompletionItem#label
     */
    label2?: CompletionItemLabel;
}

export interface CompletionItemLabel {

    /**
     * The function or variable
     */
    name: string;

    // The signature, without the return type. is render directly after `name`
    // signature?: string; // parameters
    // The fully qualified name, like package name, file path etc
    // qualifier?: string;

    // The return-type of a function or type of a property, variable etc
    type?: string;
}

We use label2 so we can isolate changes to proposed API.
In the end label will be string | CompletionItemLabel.

When you provide label2 with type, it'll look like this:

suggest

@octref Keeping this item alive for the API discussion/process (since it has all the history)

Current state of UI, with Status Bar turned on:

image

Implementation:

vscode.languages.registerCompletionItemProvider('plaintext', {
  provideCompletionItems() {
    return {
      isIncomplete: false,
      items: [
        {
          label: 'foo',
          label2: {
            name: 'foo',
            signature: '(bar)',
            qualifier: 'baz.foo',
            type: 'MyType'
          },
          documentation: 'my documentation',
          detail: 'my detail'
        },
        {
          label: 'foo2',
          label2: {
            name: 'foo2',
            signature: '(bar)',
            qualifier: 'baz.foo',
            type: 'MyType'
          },
          documentation: 'my documentation',
          detail: 'my detail'
        }
      ]
    }
  }
})

Summary of discussion with TS Team:

Thoughts about current proposal

  • We're concerned about the size of the completions object. A good number of TS completions responses return more than 1000 items. Adding even a single string field to these means a lot more data is going back and force between VS Code and TS Server

  • TS cannot compute the signature without doing additional work. Again, we don't want to perform this 1000+ times for a completion

  • However, TS would be able to return some minimal additional information for a small set of suggestions. For example, we could eagerly return the auto import source file name since we already have this info and auto import suggestions are usually a small percent of the total number of suggestions returned

Ideas

  • It'd be much better if we could lazily resolved the label for the visible items. In fact, we already have a TS API that can more or less do this. I know you are not going to like to hear that, but for JS/TS we cannot eagerly compute all the label information up front. With the current proposal, it seems like we will only be able to support returning at most one of the new fields for a handful of completion items

The current API doesn't exclude resolving the label later. I think it's reasonable to only return a string label and later return a CompletionItemLabel with type-related information.

We could say, yeah, if your provider is fast and can provide everything upfront, do it and we show info. Otherwise, as user goes up/down, we resolve each item, show info and do not hide the info on changing selection. But I guess that wouldn't be what users want exactly.

So we are in a dilemma:

  • Providers cannot resolve all items upfront as it's too expensive
  • Providers have no control over what's the first set of items displayed, so they cannot eagerly resolve them
  • We can't change our resolve behavior to resolve all shown items (or can we?). But as users type to filter items, that would bombard the providers with 10x resolve requests...Maybe only do that when user stop typing for a period of time?
```
- user stop typing
- send resolve for selected item
- if there's no input for 150ms
- send resolve for shown, but unselected items
```

I know you are not going to like to hear that, but for JS/TS we cannot eagerly compute all the label information up front.

This isn't about likes or not but about how well does a feature work or not. Already today with our code-base we suffer from this, just trigger completions for Range and 馃挬. I do believe that TypeScript needs to come up with an answer for this.

Screenshot 2020-02-06 at 10 54 39

I understand the challenge that TS is facing but I think it needs a real, fresh look and things need to be measured (before judgement) and some things might need refactoring:

We're concerned about the size of the completions object. A good number of TS completions responses return more than 1000 items. Adding even a single string field to these means a lot more data is going back and force between VS Code and TS Server

This needs measurements! When I run "F1 > Measure Extension Host Latency" then I see 500-1000Mb/s which hints to me that this might not be the biggest problem here. Then, what are those 1000+ items, surely not member-completion items or locals but likely mostly globals that aren't changing for the whole project. TSServer could compute such static results once and only tell the extension when to suggest them or not. I have a feeling that would help a lot. Last, TS can always experiment with returning incomplete result sets.

Anyways, let's not derail this issue further and lets discuss TS specific issues in https://github.com/microsoft/vscode/issues/88757 and the TS mirror issue, this issue is about the API and UX and for extensions that already today have this information (like Java)

Sorry but this is not derailing: I shared real feedback on this proposal from our number one language (by user number). And while there are potential workarounds, we need to take this feedback into account.

I can setup a meeting with TS if you wish to discuss challenges of the proposal and what TS should ideally provide to VS Code for completions.

I say derailing because there is #88757 to discuss the TypeScript aspect of this. I believe that we already have languages that can implement this, like Java, php, and python (?), and that we should get feedback from them. And yeah, saying let's implement "always showing details" but not doing that isn't constructive and yeah, I'd love to discuss this with TypeScript folks directly.

In the Haxe extension, we currently include the package path of types in the completion item label similar to Java. The main reason we do this is so that you can _filter_ by the fully qualified path (related issue: #67887).

Will CompletionItemLabel.qualifier be considered in filtering, or is it purely cosmetic?

Note: yes, there already is a separate filterText that could be used, but I think the qualifier would have to support the same "filter highlight UI" (matched characters being highlighted in blue) that the label currently has to make for a good user experience.

@jrieken you can add Metals (the Scala language server) to the list. We would support this feature right away if it became available via LSP

Will CompletionItemLabel.qualifier be considered in filtering, or is it purely cosmetic?

Yes - the current design is that only label is being used for filter/highlighting and that the other properties are used for decoration and structure. Adding the qualifier is an interesting idea that's worth being explored but that also adds semantics, e.g. the qualifier must be the long-form of the label (which is our design but we have no plans to enforce that)

For the current proposal there are a few things I am unsure about and would like feedback

export interface CompletionItemLabel {
    // the name of the function or variable
    name: string;
    // the signature, without the return type. is render directly after `name`
    signature?: string; // parameters
    // the fully qualified name, like package name, file path etc
    qualifier?: string;
    // the return-type of a function or type of a property, variable etc
    type?: string;
}
  • the signature property is dubious because (1) a signature is usually parameters and return type but we only want parameters and (2) our rendering is always like <name><signature> which means we assume all languages have this sequence. We cannot change the rendering but add a little fuzziness (this picks up some @octref's initial ideas) , e.g. call it nameDetailsor nameAppendix and then we'd recommend to populate it with parameters when applicable
  • to express the intend of qualifier better we could use qualifiedName

To me, qualifiedName implies that it's the full name, so it would usually include the label. That seems redundant / like it would waste horizontal space in the UI.

Whereas the current qualifier would only be the prefix you have to add to qualify the name / label.

@mjbvz @jrieken鈥攆rom the TypeScript side, would the ideal response include a separate completion entry for every overload of the same method/function? Currently, we only send one entry per identifier, and then when we send the details, we show the first overload and a string indicating how many additional overloads (if any) are available:

image


Edit: feel free to move to the TS-specific issue I had lost track of; https://github.com/microsoft/TypeScript/issues/36265 鈥攃ross posting there.

Made the change from CompletionItemlabel#signature to CompletionItemlabel#parameters, so the name can indicate lack of return type.

I'm often checking if this is still in progress. Maybe at least we can have an API from VS Code side so we can implement it in our completion providers (other extensions like Dart, Java, etc.) and then TypeScript will catch up?

@jrieken As far as I understand API is not stable yet and it's in proposal? If it's in proposal and not yet stable, how we can help to move things forward? I know there are other work to do, but this change would improve DX very very much!

Yes this is a little unfortunate because in contrast to others TypeScript seems unable to implement this (see https://github.com/microsoft/TypeScript/issues/36265#issuecomment-673126687). We could ignore them or revisit - tho updating items incrementally while showing them (understand flicker) isn't something we aim for. The other unfortunate part is that the dev that worked on this left the team which means this is currently in the air.

@jrieken Thanks a lot for info. I'm happy you're aware of this issue and it may be implemented in the future and that's enough for me - the hope. 馃檹

Was this page helpful?
0 / 5 - 0 ratings

Related issues

chrisdias picture chrisdias  路  3Comments

borekb picture borekb  路  3Comments

lukehoban picture lukehoban  路  3Comments

curtw picture curtw  路  3Comments

biij5698 picture biij5698  路  3Comments