Skip to content
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

x/tools/gopls: consider using &S form instead of *S for pointer type struct field autocompletion #39227

Open
hyangah opened this issue May 23, 2020 · 5 comments
Labels
gopls Issues related to the Go language server, gopls. help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.

Comments

@hyangah
Copy link
Contributor

hyangah commented May 23, 2020

Currently autocompletion for a struct field uses the field's type as the placeholder text. For example,

type F struct {...}
type S struct {
   Field *F
}

The autocompletion for the Field will return "newText":"Field: ${1:*F},".
gopls uses the type info for placeholder text, so I don't think the choice is incorrect nor invalid.

But if the suggestion used &F instead, it would be more convenient when users want to use composite literals. Especially, if the type name is long to type, reusing what's already in the placeholder will save many keystrokes.

(Based on golang/vscode-go#85)

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels May 23, 2020
@gopherbot gopherbot added this to the Unreleased milestone May 23, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 May 26, 2020
@stamblerre
Copy link
Contributor

/cc @muirdm

@muirdm
Copy link

muirdm commented May 27, 2020

This was discussed briefly in #38416, but we should consider getting rid of the struct field placeholder text altogether. It is weird to insert invalid, informational Go code when most editors already present the type information to the user.

Composite literals prepend the "&" for you automatically as needed, and sometimes you don't want a composite literal, so I don't think having "&" placeholder text is a net win.

@hyangah
Copy link
Contributor Author

hyangah commented May 27, 2020

@muirdm Interestingly I found the struct field placeholder is useful when dealing with badly named long type names with common prefixes, even though I dislike the current way of formulating function parameter place holders.

I don't know about other editors, but at least, in VS Code the suggested text syntax is followed. So if I don't want a composite literal, just typing the other value at the cursor completely replaces the entire suggested text with my input. Actually, that's easier than using the suggested text.

VS Code presents the type info as a hover message, which means I have to hover over the field to remember the type name to trigger meaningful autocompletion results. Additional cognitive overhead. It would be convenient if &someproto.VeryLongMessage{} was suggested. (And, syntax-wise, it's a valid syntax)

As a workaround, I hoped just typing & triggered completion and suggested the suitable composite literal snippet for the field, but it doesn't seem like VS Code ever triggers autocompletion with this symbol (&) for a reason I don't know. :-(

BTW I didn't encounter automatically prepended "&". Is there any configuration I need to set to enable it? Or, do you mean the fact that the completion response includes &F{} as one of the candidates?

@muirdm
Copy link

muirdm commented May 27, 2020

It would be convenient if &someproto.VeryLongMessage{} was suggested.

Yes, that would be convenient in many cases. And of course it also contains the type information of the current placeholder.

As a workaround, I hoped just typing & triggered completion

You can also try triggering completion without typing anything at all (ctrl-space in VSCode I think). That will often pop up the literal candidate. I think you can use ctrl-space to trigger completion after "&" as well. Perhaps you can configure VSCode's "trigger" characters to include "&".

I didn't encounter automatically prepended "&" ... do you mean the fact that the completion response includes &F{}

Yes, the candidate will include "&" if it needs to be a pointer. In your example you could type "Very" and probably get a candidate for "&someproto.VeryLongMessage{}".

@stamblerre how do you complete a long composite literal for a struct field value when you don't remember the name? Do you rely on hover popup, trigger completion manually, or something else?

@stamblerre
Copy link
Contributor

@stamblerre how do you complete a long composite literal for a struct field value when you don't remember the name? Do you rely on hover popup, trigger completion manually, or something else?

Ah, so I was confused by this and now I see why this has never been an issue for me. I don't use placeholders, so when I complete a struct literal, it looks like this:

Screen Shot 2020-05-27 at 2 51 57 PM

(This image shows a completion triggered at Field: <> inside of the S struct literal, with the first option being &F{}.)

I also trigger completion manually a lot with Ctrl + Space. This flow works well for me, but I could see it being hard with placeholders. I would support having the placeholder be an &veryLongName{<>} with the <> marking where the user's cursor ends up. I'll turn on placeholders for a few weeks to learn more, and obviously I defer to the opinions of anyone who does use placeholders.

@stamblerre stamblerre changed the title x/tools/gopls: consider using &S form instead of *S for pointer type struct field autocompletion. x/tools/gopls: consider using &S form instead of *S for pointer type struct field autocompletion Jun 24, 2020
@stamblerre stamblerre removed this from the gopls/v0.5.0 milestone Jun 24, 2020
@stamblerre stamblerre added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jul 23, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants