Functions auto completion on combinators could have better info text

[FMorschel]

If you have the following code:

import 'dart:math' show l^

This is what code completion looks like.

20250211_081458.mp4

It does know we are inside a combinator and it doesn't add the braces. But to me, this feels like when you are completing it as a call. When you do complete log as a call (inside a body for example), it adds (x).

I'd like to ask, similarly to #59929, that we change the info here to something like function. So it doesn't feel like it will add unwanted braces.

I'm not sure this is enough of a reason to change, but it feels a bit off that the info is the same but the completion is not.

[DanTup]

I'm unsure about this, or at least doing it only here.. The (...) on the end was always intended as an indicator of "this is a method". If we changed this to instead say function (similar to get/set), I think it would make me question why we don't also do that for completion in other contexts too, since the goal is the same.

[FMorschel]

Maybe changing it all makes sense. I'm mostly questioning the use of (...) when we are only completing the identifier and not adding the parameter:

• Combinators
• Comment references
• Tear offs

But as you said, if adding (...) was only for indicating that this identifier is a function and not to represent the completion necessarily, than maybe changing it all makes sense for cohesion.

@bwilkerson @scheglov @srawlins WDYT?

[bwilkerson]

It makes sense to me to make a distinction between completing a function/method as

• an invocation of the function
• a tear-off
• a non-expression reference to the name

I think we're already making the distinction for the first two cases (haven't confirmed), so adding the third seems reasonable.

[DanTup]

I'm not sure we currently differentiate the first two either:

void myFunc() {}

void myFunc2(void Function() a) {}

void f() {
  myFunc2(my^)
}

Here, we show myFunc like a method:

But we complete only the name:

I'm conflicted about this though. On one hand, I like that we can tell it's a method/function (although I'll note that the icon is also different for methods), but on the other hand I can see how it might surprise you that completing it didn't include trailing parens.

We do usually show the type of entries in completion, for example see myLocalFunc here:

So I wonder if we should do the same here (eg. show it as "myFunc()" for invocations and "myFunc void Function()" for tear-offs). Probably the reference to the name should be the same as the tear-off?

[FMorschel]

We do usually show the type of entries in completion, for example see myLocalFunc here

That is what I was thinking. I also feel like functions and methods are treated differently than the FunctionType variables and even though I get the reason, it also feels a bit weird.

So I wonder if we should do the same here (eg. show it as "myFunc()" for invocations and "myFunc void Function()" for tear-offs).

I like that better than the current state, but then for FunctionType variables we would probably always show the longer option since is the type of the variable and this way we would not be able to know when we want to show (...). It's better anyway, just thinking about coherence across things too.

Probably the reference to the name should be the same as the tear-off?

Agreed on my part.

[bwilkerson]

That all sounds reasonable to me.

[DanTup]

I went back and forth a few times while trying to implement this in a consistent way, and my ultimately conclusion is that probably the only difference here is that we want a space before the arguments. For something that is callable and an invocation, we but the params right up against the label to make it clear you're calling it:

But when you're not calling it, we show essentially the same thing (the type, or the signature in the form (args) -> returnType) but with a space before it because it's just extra information and not a call. So if we just include a space in the case where it's not an invocation, it looks like this:

We could show the full type (without truncating params), but I think that would be a little inconsistent (while we do this for function types, it's more accidental than anything, because we just get them as a string). We could also show the label in the form returnType Function(args), however this takes up more space (which we don't have a lot of here) and I think the current form is more consistent with how we usually show callable things (although it's different to how we'd show function types).

Any opinions?

(as an aside, I was wondering it we should only use … with the parameters are above a certain threshold, like 20-30 chars, so in the case of min(a, b) we would show the args? It does mean there's this weird magic number, but it might also be nice if we don't hide the parameters where it wasn't really necessary - for some blurry definition of necessary).

[DanTup]

Here's what the change for that would look like (which preserves all over existing behaviour/tests passing): https://dart-review.googlesource.com/c/sdk/+/411022

[FMorschel]

I like the current approach, yes! Thanks for taking a look at this.

---

as an aside, I was wondering it we should only use … with the parameters are above a certain threshold, like 20-30 chars, so in the case of min(a, b) we would show the args

I'd love this! I'd only say that we should probably avoid showing named parameters here since they usually get big with required before them, so maybe only doing an {...} might suffice? No really hard feelings on this either way so if you think we should still show them its fine by me.

One thing that I'd say it would be better is if you could get a substring for them in that length and add ... after if they are longer. And on this note if you know positional parameters will take more than that magic number of length and you also have named, I'd say we should addd both like: ..., {...} but it may not be necessary. It's just to be more coherent but again, no hard feelings here either.

I don't think optional positional should get the same treatment since they are optional but maybe?

[DanTup]

Given you can see the full signature by having the item selected, I think if we do anything we should keep it very simple. I was just thinking for the cases where the parameters are really short (print, min, etc.) that maybe it's worth showing them, something like doing this:

var truncatedParameters = switch (parameters) {
  null || '' => '',
  '()' => '()',
  _ when parameters.length < 30 => parameters, // Add this line here
  _ => '(…)',
};

It's a trade-off between consistency and some additional usefulness in some specific cases. I'm not sure how I feel about it though.