x/tools/gopls: add location-aware snippets for append and other utility methods #39507
Comments
@A-UNDERSCORE-D Yeah, Goland's context-aware, custom postfix completion support is nice. As you mentioned, we want to delegate most completion logic to the language server and this is the tracking issue #39354. The completion snippets around |
Thanks for the suggestion, @A-UNDERSCORE-D. In general, this does sound like a We actually should have some similar functionality around |
Thanks for the transfer! And yeah Ive noticed append popping up like that on occasion, it works quite well. I guess my real want here is a more direct way to tell gopls "please give me this specific autocompletion" as shown in #39354 already. Not sure if I should keep this open or defer to the #39354? I think mine provides a bit more abstract around expected behavior for postfix style completions. That said #39354 may be an opening for something more abstract where there can be a predefined set of postfixes that can be expanded on via config if a user desires. |
Yeah, I agree this can be a more general issue. Let's start with seeing how we can improve |
FYI the CL (https://go-review.googlesource.com/c/tools/+/221777) that implements autocompletion for @A-UNDERSCORE-D when is the second command 'assign append inplace' rule useful? (We are trying to understand use cases and how to improve the autocompletion logic) |
@hyangah Its mostly just an ease of use thing I picked up from my time using goland. Its a part of the whole postfix completion request. For us with gopls the main thing I can think of its its an unambiguous way to ask gopls to provide the completion we want. Meaning that instead of typing the partial name of a slice and using arrow keys to grab the append (I have not tried 4.2 yet, so the arrow keys comment is purely speculative) meaning that one's right hand needs to move to the right for arrow keys, one can use the progression partial name -> .aa (suggested append at this point) to get the same result. Or that is the intent, anyway |
I'm not getting completions for the case |
Strange, it works great in Emacs
Candidates from gopls:
Should I open a separate (VSCode?) issue @stamblerre? |
Oh, I actually didn't realize that was supposed to work - thanks for clarifying. I just opened golang/vscode-go#441 - we should probably investigate it in VS Code Go first, but you're right in thinking it may be a VS Code issue. |
@muirdm What's the vscode version? The candidate appears in my vscode. Maybe I am missing something. |
I'm playing around with postfix completions if anyone wants to try it and give feedback: https://go-review.googlesource.com/c/tools/+/272586 You can see the list of postfix thingies available here: https://go-review.googlesource.com/c/tools/+/272586/1/internal/lsp/source/completion/postfix.go#82 The list is not necessarily complete, and the things in the list are not necessarily useful enough to be in the list. I was just trying a variety of things to see how they would work. |
Playing around with it a bit now. I really like this. the ! indicates its a little snippet, which is very helpful. And what you have is quite nice, I'd probably use them all over time. Only complaint is that there arent docs sent with it. Not sure if that can be changed or not. |
is there a way I can add suggestions or otherwise to your CL @muirdm? Ive been considering implementing some extra ones |
You can add things to the list in https://go-review.googlesource.com/c/tools/+/272586/1/internal/lsp/source/completion/postfix.go#82 and then rebuild gopls. If you mean push changes to my branch, then I don't know. I assume there is some way to collaborate like that. My branch is rough and likely to change a lot, so I wouldn't put in too much time.
Do you mean more detail showing what the snippet will do, or actual textual documentation explaining it what it will do? |
yeah, a "This is what you'll get if you use this" kind of idea. IIRC vscode can show a little box next to the completion option? Im not 100% sure but I think that its a thing |
Yes, gopls can fill in the completion item "detail" for that I think. |
Poked around a bit in that, was pretty easy. Still getting used to how gopls works internally. I think this is reasonable though. Also your code appeared to rebase fine to the latest release, but not to master -- I definitely dont know enough to fix the conflict though -- Not sure how to resolve the two different edit methods. diff --git a/internal/lsp/source/completion/postfix.go b/internal/lsp/source/completion/postfix.go
index 1f95cff2..3eb62e67 100644
--- a/internal/lsp/source/completion/postfix.go
+++ b/internal/lsp/source/completion/postfix.go
@@ -71,6 +71,7 @@ func and(filters ...postfixFilter) postfixFilter {
type postfixRule struct {
name string
+ details string
filter postfixFilter
label string
imports []string
@@ -84,52 +85,60 @@ var postfixRules = []postfixRule{{
filter: isSLice,
label: "sort",
imports: []string{"sort"},
+ details: "Insert a sort.Slice implementation",
body: `{{.Import "sort"}}.Slice({{.Obj}}, func(i, j int) bool {
{{.Cursor}}
})`,
}, {
- name: "slice_last",
- filter: isSLice,
- label: "last",
- body: "{{.Obj}}[len({{.Obj}})-1]",
+ name: "slice_last",
+ filter: isSLice,
+ label: "last",
+ body: "{{.Obj}}[len({{.Obj}})-1]",
+ details: "Access the last item in this slice",
}, {
- name: "slice_reverse",
- filter: isSLice,
- label: "reverse",
+ name: "slice_reverse",
+ filter: isSLice,
+ label: "reverse",
+ details: "Reverse this slice (in-place)",
body: `for i, j := 0, len({{.Obj}})-1; i < j; i, j = i+1, j-1 {
{{.Obj}}[i], {{.Obj}}[j] = {{.Obj}}[j], {{.Obj}}[i]
}`,
}, {
- name: "slice_range",
- filter: isSLice,
- label: "range",
+ name: "slice_range",
+ filter: isSLice,
+ label: "range",
+ details: "Iterate over this slice",
body: `for i, {{index .Vars 0}} := range {{.Obj}} {
{{.Cursor}}
}`,
}, {
- name: "slice_append",
- filter: and(isSLice, isAssignable),
- label: "append",
- body: `{{.Obj}} = append({{.Obj}}, {{.Cursor}})`,
+ name: "slice_append",
+ filter: and(isSLice, isAssignable),
+ label: "append",
+ details: "Append to this slice",
+ body: `{{.Obj}} = append({{.Obj}}, {{.Cursor}})`,
}, {
- name: "map_range",
- filter: isMap,
- label: "range",
+ name: "map_range",
+ filter: isMap,
+ label: "range",
+ details: "Iterate over this map",
body: `{{$v := .Vars}}for {{index $v 0}}, {{index $v 1}} := range {{.Obj}} {
{{.Cursor}}
}`,
}, {
- name: "map_clear",
- filter: isMap,
- label: "clear",
+ name: "map_clear",
+ filter: isMap,
+ label: "clear",
+ details: "Clear this map",
body: `{{$k := (index .Vars 0)}}for {{$k}} := range {{.Obj}} {
delete({{.Obj}}, {{$k}})
}
`,
}, {
- name: "map_keys",
- filter: isMap,
- label: "keys",
+ name: "map_keys",
+ filter: isMap,
+ label: "keys",
+ details: "Create a slice of all the keys in this map",
body: `keys := make([]{{index .Types 0}}, 0, len({{.Obj}}))
{{$k := (index .Vars 0)}}for {{$k}} := range {{.Obj}} {
keys = append(keys, {{$k}})
@@ -145,24 +154,28 @@ var postfixRules = []postfixRule{{
name: "print_scalar",
filter: isScalar,
label: "print",
+ details: "Print the %v of this variable",
imports: []string{"fmt"},
body: `{{.Import "fmt"}}.Printf("%v\n", {{.Obj}})`,
}, {
name: "print_multi",
filter: isMulti,
label: "print",
+ details: "Print all of these",
imports: []string{"fmt"},
body: `{{.Import "fmt"}}.Println({{.Obj}})`,
}, {
name: "warn_scalar",
filter: isScalar,
label: "warn",
+ details: "Print this to stderr",
imports: []string{"fmt", "os"},
body: `{{.Import "fmt"}}.Fprintf({{.Import "os"}}.Stderr, "%v\n", {{.Obj}})`,
}, {
name: "warn_multi",
filter: isMulti,
label: "warn",
+ details: "Print all these to stderr",
imports: []string{"fmt", "os"},
body: `{{.Import "fmt"}}.Fprintln({{.Import "os"}}.Stderr, {{.Obj}})`,
}}
@@ -341,6 +354,8 @@ Rules:
Kind: protocol.SnippetCompletion,
snippet: snip,
AdditionalTextEdits: edits,
+ Documentation: snip.String(),
+ Detail: rule.details,
})
}
} |
I've updated the cl to push more logic into the completion templates. This way gopls can theoretically support dynamic user supplied postfix completion templates specific to the user's code base. The template framework probably needs a few more facilities to make this actually useful (for example a way to test for a specific type, e.g. I also added basic "details" description for each completion. I don't think including the snippet text as the documentation is a good idea since that would include inscrutable snippet meta information, depending on the snippet. TODOs:
@stamblerre can you take a preliminary look and either bless or condemn the general approach? |
Change https://golang.org/cl/272586 mentions this issue: |
I've pushed a more polished CL to drive things forward. Most of the issues I know about are resolved. I'm curious to hear what others think of this feature. Personally I doubt I will remember to use them, and most of the snippets don't really compel me. |
Built, I'll play with it sometime when I next write go. And yeah its very much not for everyone, its just how my thought process works, I'm typing as I think, so "use x and do ..." is where I'm going |
Is your feature request related to a problem? Please describe.
After using GoLand for some time I'm missing a small feature or two. Specifically I'm missing type aware snippets for append, sprintf, and a few other utility methods (like things from slicetricks).
Describe the solution you'd like
I'd like a snippet under the name
.aapend
(auto-append) or similar to construct an append-assign statement for me.Either from the start by starting with
append
and gettingtoAppend = append(toAppend, <>)
Or from the middle of a line using something like
.aapend
which would add the correct= append(name, <>)
to the end of the line.The sprintf and slicetricks I mentioned above would work similarly, sprintf simply going from
thing.sprint
tofmt.Sprintf("<>", thing)
, and delete applying a (preferably pointer aware delete implementation from the slicetricks wiki pageDescribe alternatives you've considered
I've written my own snippets for this, but they get suggested in comments and other places where they are not useful, cluttering any autocomplete I may have had. And these rely on somewhat brittle regexps, which do not always get it right.
The other option here is to type things manually, though that has the annoyance that for some things, you're going to end up checking a wiki page EVERY time (slicetricks things, delete et al.) or are just repetitive--typing
thing = append(thing, newStuff)
gets old. Not to mention that if its type aware it also suggestthing = append(thing, newStuff...)
where appropriate.This may be more applicable to gopls itself, rather than here, but I thought I'd ask about it here first.
The text was updated successfully, but these errors were encountered: