x/tools/gopls: completion should also provide just the function name

[pjweinb]

gopls version

v0.14.2

go env

irrelevant

What did you do?

I have an existing expression f(a), and I would like to replace it with f(protocol.FWithLongName(a)). After typing protocol.F, the only useful completion offered is the function snippet, which would complete as
f(protocol.FWithLongName(x protocol.TypeWithLongName, y TypeOfSecondArg)(a))
which requires too much editing.

What did you expect to see?

I would like to have seen both the function snippet and just the function names as suggested completions.

What did you see instead?

The unsyntactic completion described above. gopls is not looking at the context when it proposed a snippet where there is already an argument.

Editor and settings

No response

Logs

No response

[findleyr]

Does "completeFunctionCalls": false do what you want?

[pjweinb]

Sometimes i want the whole thing, sometimes I want just the name, depending on my editing need at the moment. I'd like much more control, actually, but can't envision a suitable user interface. I want 'false' for printf-like functions, 'true' for non-varargs functions if I'm editing at the end of an expression or statement, and 'false' if i'm editing in the middle of an expression. Providing both versions of the function completion seems easier.

…

On Thu, Nov 30, 2023 at 3:41 PM findleyr ***@***.***> wrote: Does "completeFunctionCalls": false do what you want? — Reply to this email directly, view it on GitHub <#64469 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJIAI5P76S6GETHKOX2UZLYHDVQ7AVCNFSM6AAAAABABK6YY6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZUGUZTCMBWGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[hyangah]

I have an existing expression f(a), and I would like to replace it with f(protocol.FWithLongName(a)). After typing protocol.F, the only useful completion offered is the function snippet, which would complete as
f(protocol.FWithLongName(x protocol.TypeWithLongName, y TypeOfSecondArg)(a))
which requires too much editing.

I agree that the completion logic needs to be smarter in this case.

Can you share your gopls setting? I disabled usePlaceholder for the similar issue at some point in the past.

[pjweinb]

Turning it on and off with a setting would mean changing setting frequently while editing. Giving both choices seems better.

…

On Wed, Dec 6, 2023 at 10:15 PM Hyang-Ah Hana Kim ***@***.***> wrote: I have an existing expression f(a), and I would like to replace it with f(protocol.FWithLongName(a)). After typing protocol.F, the only useful completion offered is the function snippet, which would complete as f(protocol.FWithLongName(x protocol.TypeWithLongName, y TypeOfSecondArg)(a)) which requires too much editing. I agree that the completion logic needs to be smarter in this case. Can you share your gopls setting? I disabled usePlaceholder for the similar issue at some point in the past. — Reply to this email directly, view it on GitHub <#64469 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJIAIYAQXL2ZHO2IA26SRTYIEYENAVCNFSM6AAAAABABK6YY6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBUGE4DOMJTGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[hyangah]

@pjweinb i am not suggesting users have to turn on/off the settings while editing. I meant usePlaceholder setting became counter-productive due to this problem and I personally don't use that setting any more. Giving both option will be confusing unless we figure out good presentation, and make the suggestion list longer.

[pjweinb]

perhaps we should have a counter to see what fraction of our users use 'usePlaceholder'. If hardly anyone does, then it could be removed. It seems we have no principled way of deciding about what sort of settings to have, other than our individual personal valuing of user suggestions.

…

On Thu, Dec 7, 2023 at 8:31 AM Hyang-Ah Hana Kim ***@***.***> wrote: @pjweinb <https://github.com/pjweinb> i am not suggesting users have to turn on/off the settings while editing. I meant usePlaceholder setting became counter-productive due to this problem and I personally don't use that setting any more. Giving both option will be confusing unless we figure out good presentation, and make the suggestion list longer. — Reply to this email directly, view it on GitHub <#64469 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJIAI5XW5ULTFSNR3IOXRLYIHALLAVCNFSM6AAAAABABK6YY6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBVGM2DQOJZGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[muirdm]

I wonder if we could try to make the snippet smarter. In f(a) where you want f(other(a, ...)), if a can be passed to other, maybe we could see that and offer other(a, <nextArg>) as the completion. Or maybe that is too specific.

Anyway, in cases where there is a trailing identifier after the cursor, I feel it is reasonable to also offer the naked function call as a completion candidate since it is hard to tell what the user wants.