Skip to content

Always show return type/signature for all items in code completion #2462

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

Closed
RaviKavaiya opened this issue May 16, 2020 · 18 comments
Closed

Always show return type/signature for all items in code completion #2462

RaviKavaiya opened this issue May 16, 2020 · 18 comments
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is enhancement
Milestone

Comments

@RaviKavaiya
Copy link

Is it possible to show return type of a method in intellisense (or a complete method signature)?

Sometimes, it is much useful when we have a large number of methods available. And to pick one quickly, this will help much.

For example, in Android Studio, it shows like this:
Screenshot 2020-05-16 at 7 57 32 PM

@DanTup
Copy link
Member

DanTup commented May 18, 2020

We do show the return type in the same place here:

Screenshot 2020-05-18 at 10 09 24

However VS Code only shows it for the highlighted item. There's an open issue about improving this here:

microsoft/vscode#39441

If that is implemented, we should be able to improve this here too.

@DanTup DanTup added this to the On Deck milestone May 18, 2020
@DanTup DanTup added blocked on vs code / lsp / dap Requires a change in VS Code to progress in editor Relates to code editing or language features is enhancement labels May 18, 2020
@DanTup DanTup modified the milestones: On Deck, Backlog Jul 2, 2020
@DanTup
Copy link
Member

DanTup commented Aug 26, 2021

Looks like the LSP part of this work will be in v3.17 of the spec.

microsoft/language-server-protocol#1307

@DanTup DanTup modified the milestones: Backlog, On Deck Aug 26, 2021
@DanTup DanTup added the in lsp/analysis server Something to be fixed in the Dart analysis server label Aug 26, 2021
@DanTup DanTup modified the milestones: On Deck, v3.48.0 Jul 26, 2022
@DanTup DanTup removed the blocked on vs code / lsp / dap Requires a change in VS Code to progress label Jul 26, 2022
@DanTup DanTup modified the milestones: v3.48.0, v3.50.0 Aug 25, 2022
@DanTup DanTup modified the milestones: v3.50.0, v3.52.0 Oct 3, 2022
@DanTup DanTup modified the milestones: v3.52.0, v3.54.0 Oct 31, 2022
@DanTup DanTup modified the milestones: v3.54.0, v3.56.0 Nov 28, 2022
@DanTup DanTup modified the milestones: v3.56.0, On Deck Dec 12, 2022
@DanTup DanTup modified the milestones: On Deck, v3.62.0 Feb 27, 2023
@DanTup DanTup modified the milestones: v3.62.0, v3.64.0 Mar 23, 2023
@DanTup DanTup modified the milestones: v3.64.0, v3.66.0 Apr 4, 2023
@DanTup DanTup added this to the v3.68.0 milestone May 16, 2023
@DanTup DanTup changed the title Possible to show method return type? Always show return type/signature for all items in code completion May 24, 2023
@DanTup
Copy link
Member

DanTup commented May 24, 2023

Looking for some opinions on including signatures more visible in code completion (today, only the item your cursor is over shows the signature, and we can't show greyed labels on the left, so the parens on each item is quite strong).

Today (without changes):

Screenshot 2023-05-24 at 18 17 24

Possible changes:

  1. Full signature, split between left/right
  2. Full signature on left
  3. Full signature on right
  4. Abbreviated params (...), split between left/right
  5. Abbreviated params (...) on left
  6. Abbreviated params (...) on right

Screenshot 2023-05-24 at 17 50 56
Screenshot 2023-05-24 at 17 51 49
Screenshot 2023-05-24 at 17 52 24
Screenshot 2023-05-24 at 17 54 27
Screenshot 2023-05-24 at 18 08 26
Screenshot 2023-05-24 at 17 57 55

I think I prefer 2 or 5. 2 is a bit noisier, but seeing the parameters (when there aren't too many) is nice. It also slightly better matches the descriptions of these two labels in the LSP spec (the "detail" label which is on the left says function signatures and type annotations):

  /// An optional string which is rendered less prominently after
  /// [CompletionItem.detail]. Should be used for fully qualified names and file
  /// paths.
  final String? description;

  /// An optional string which is rendered less prominently directly after
  /// [CompletionItem.label],
  /// without any spacing. Should be used for function signatures and type
  /// annotations.
  final String? detail;

Another thing to note is that we used to show "Auto import from ..." on the right side for the selected item, but now we would either show them all the time, or only if you expand the panel to the right:

Screenshot 2023-05-24 at 18 18 59

We can't mix both the new labels and have the old one show up for the "selected" item.

@bwilkerson @jacob314 @InMatrix

@bwilkerson
Copy link

Possible changes:

I can provide my opinion, but I don't have any data to back it up.

My preference would be for option 5:

  • I find it harder to read when the signature is split between the left and the right, so I dislike 1 and 4.

  • I more often care about the return type than the parameter types because I'm usually concerned with how to get the data I want, and will figure out later how to provide all of the arguments that are required. Including all of the parameter information sometimes causes the return type to be truncated, so I consider that to be a downside. Given that the documentation pop-up will include the full signature, I think it would be best to occlude the parameters.

  • Putting the information on the left causes it to be in a larger font, so the information will more often be truncated, but even so I prefer having the member name with the signature makes it more readable.

... we used to show "Auto import from ..." on the right side for the selected item, but now we would either show them all the time, or only if you expand the panel to the right:

I have a slight preference for putting them in the panel to the right because the main presentation is less crowded that way and I rarely need that information. But the advantage of showing them always is that it's less work to get the information when it's needed, so I could see a productivity boost when the data is useful.

@DanTup
Copy link
Member

DanTup commented May 30, 2023

Thanks - I think I agree. I'll wait a little longer to see if there are any other opinions but otherwise may go ahead with 5 soon (and we can always refine if we get other feedback in future).

@DanTup DanTup modified the milestones: v3.68.0, v3.70.0 Jun 28, 2023
@svipas
Copy link

svipas commented Jun 29, 2023

I vote for 4. It is aligned with how other editors do it and VS Code as well in JS/TS env at least, but I think other languages follow the same. I just think that you should remove () and keep only (...), because () adds noise without actual value.

1 is good as well, but if params are too long we lose ability to see full return type and for some it could be too noisy. That's why I think it should be trimmed and have max length. This way we see some info about params as well as full return type.

In my opinion this could be under config with ability to change between 1 and 4. paramsWithReturnType & onlyReturnType (default).

@DanTup
Copy link
Member

DanTup commented Jul 5, 2023

I vote for 4. It is aligned with how other editors do it and VS Code as well in JS/TS env at least,

I think TS is currently doing what Dart is doing (which not using this new functionality). It has the signature in the "detail" field (because it's the only place to put it) which is only visible for the current item (and shows on the right if you have the detail pane collapsed):

image

image

This is what Dart is doing today (in part because we can't show it on the left without it being the same style as the name). I think if TS adopts the new APIs, it would probably put the signature on the left, because the VS Code APIs describe the fields in that way (in the new API, detail is on the left and is documented as being for signatures, and description is on the right which it says is for qualifying names or file paths):

https://code.visualstudio.com/api/references/vscode-api#CompletionItemLabel

I just think that you should remove () and keep only (...), because () adds noise without actual value.

I think there's some value in () because it shows that item is callable at a glance.

@svipas
Copy link

svipas commented Jul 5, 2023

You're actually right, it is showed for imports and snippets only. But I still like the 4, but that's probably, because I'm more familiar with it. I think () is valuable for functions, but it also shows it for classes, but since classes in Dart could be initialized without new keyword it kinda makes sense.

@bwilkerson
Copy link

I think () is valuable for functions, but it also shows it for classes, but since classes in Dart could be initialized without new keyword it kinda makes sense.

We shouldn't be displaying parens for classes, but it makes sense for them to be displayed for constructors because constructor invocations have an argument list.

@DanTup
Copy link
Member

DanTup commented Jul 5, 2023

Yup, when you see them on a class it's really for the constructor. Class names will often appear twice in the list, once as a constructor and once as a class name. This is clearer when you have classes without accessible constructors, or constructors that take arguments:

Screenshot 2023-07-05 at 20 00 17

@svipas
Copy link

svipas commented Jul 5, 2023

Nice, thanks for info, so yeah in that case it totally makes sense to keep ().

@DanTup DanTup modified the milestones: v3.70.0, v3.72.0 Jul 27, 2023
@DanTup
Copy link
Member

DanTup commented Aug 3, 2023

For now I'm going ahead with example 5 from above, but we can tweak if it doesn't feel right when people start using it.

One thing I changed slightly though is that we'll show the import-uri on the right side of the item is not already imported. The reason for this is that if you don't have the details panel visible, you'd often see duplicated items (where they're available from multiple libraries and none are imported) and it wouldn't be obvious why. With the old functionality it should show on the right side of the completion list only for the selected item, but in the new world we can't show things only for the selected item so they'll show all the time.

So with the panel collapsed it'll look something like this:

image

And with it visible like this:

image

Unfortunately we can't tell if the panel is open, so we can't hide the URIs from the right side of the completion list in that case, so there's a little duplication, but I feel like that's the best compromise.

I'm interested in feedback though (I'll post back after the change lands, it's not complete yet).

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Aug 6, 2023
…abelDetails support

This is just a little refactoring towards Dart-Code/Dart-Code#2462. It doesn't change anything, it just makes the next CL (with the functional changes) simpler.

Change-Id: I98ed5991f1abb67f86f9d4d43c27ff6ccf21847c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/317940
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Aug 30, 2023
This allows more of the signature and URI to be shown in the full completion list without needing to cursor through each item to see it.

Some examples can be seen in Dart-Code/Dart-Code#2462 (comment)

Fixes Dart-Code/Dart-Code#2462

Change-Id: Ib8290f90c31f974271109df7912d230b5e824319
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323380
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
@DanTup
Copy link
Member

DanTup commented Aug 31, 2023

This has landed in the latest SDK (it may take a few days to get to Flutter master). Since it ships in the SDK it'll show up in a future SDK release (not a VS Code extension release). If you have any feedback/issues about the changes, please open new issues with details - thanks!

@DanTup DanTup closed this as completed Aug 31, 2023
@DanTup DanTup modified the milestones: v3.72.0, Requires SDK Release Sep 4, 2023
@DanTup DanTup modified the milestones: Requires SDK Release, v3.78.0 Oct 31, 2023
@Number-3434
Copy link

Could there be an option to enable option 2?

I really like that you can see all the parameters there.

@DanTup
Copy link
Member

DanTup commented Feb 15, 2024

@Number-3434 generally I think we should avoid too many options because it makes testing and maintaining more complicated, particularly where the combinations could multiply. Here we're already handling the original LSP format and this new format, so another option would double this to possible outcomes for each completion item. I think we should be sure there's sufficient demand to warrant the option (or just changing the default). I'd suggest filing an issue at https://github.com/dart-lang/sdk to collect comments and 👍 's.

That said, it's not quite the same - but in case you didn't have the extra panel expanded, the parameters are shown in the info on the right for the selected item:

image

And if you have signature help enabled (the default), it'll pop open if you select an item and show the parameters there too:

image

These might help if you already have a general idea of what you're looking for.

@Number-3434
Copy link

Okay, I'll leave it for now.

It just felt a bit weird that it shows abbreviated parameters, but doesn't currently show the full parameters anywhere.

@DanTup
Copy link
Member

DanTup commented Feb 15, 2024

It just felt a bit weird that it shows abbreviated parameters, but doesn't currently show the full parameters anywhere.

Not sure I agree that it doesn't show them anywhere, they're shown in the places I noted above :)

Unfortunately VS Code changed how it shows the details slightly with this new work - it used to be that it showed the first line of the details on the right side of the selected item like this:

image

However when adopting this new UI, they removed that so that it only appears in the panel on the right (not inline when the item is selected). We're still putting the full signature into the details, but it'll only be visible in the panel on the right (whereas before it showed in two places).

@Number-3434
Copy link

Number-3434 commented Feb 15, 2024

Sorry, I meant specifically the inline text after the suggestion in the suggest widget.

It makes it much quicker to browse, as you can glance at the values, as opposed to having to go through every suggestion individually.

Also, personally, I really like that it shows them even if details are on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is enhancement
Projects
None yet
Development

No branches or pull requests

5 participants