Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Make autocomplete for inline functions optional #1779

Closed
kaskavalci opened this issue Jul 9, 2018 · 6 comments
Closed

Make autocomplete for inline functions optional #1779

kaskavalci opened this issue Jul 9, 2018 · 6 comments

Comments

@kaskavalci
Copy link
Contributor

#1287 addresses the issue on autocomplete for inline functions. However when passing those functions around, autocomplete can be very annoying. Can we make this feature optional?

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jul 10, 2018

We could skip using the snippet with function parameters when the cursor is followed by a comma or ).
It wouldnt cover 100% of the cases, but should work for most common cases.

The reason we cannot do a correct fix for all cases is that we are only aware that the symbol being completed is a variable of func type. We are not aware if the cursor is inside another function call.

If that sounds good enough, then PRs are welcome.

Code pointers:

@kaskavalci
Copy link
Contributor Author

Thanks for the response. I'll submit a PR this weekend.

@doxxx
Copy link
Contributor

doxxx commented Jul 19, 2018

#1788 only appears to have fixed this when completing variables of type func. When completing the name of a standalone function, it completes with the parameter list.

For example:

func HandleRequest(w http.ResponseWriter, r *http.Request) {
}

func Register(r *mux.Router, jm *job.Manager) {
	r = r.PathPrefix("/api/v1").Subrouter()
	r.HandleFunc("/", HandleReq***CURSOR***)
}

When I invoke completion at the marked cursor position, it completes to:

func HandleRequest(w http.ResponseWriter, r *http.Request) {
}

func Register(r *mux.Router, jm *job.Manager) {
	r = r.PathPrefix("/api/v1").Subrouter()
	r.HandleFunc("/", HandleRequest(w, r))
}

However, if I start with this instead:

func Register(r *mux.Router, jm *job.Manager) {
	r = r.PathPrefix("/api/v1").Subrouter()
	handleRequest := func(w http.ResponseWriter, r *http.Request) {}
	r.HandleFunc("/", handleReq***CURSOR***)
}

Completion results in the following:

func Register(r *mux.Router, jm *job.Manager) {
	r = r.PathPrefix("/api/v1").Subrouter()
	handleRequest := func(w http.ResponseWriter, r *http.Request) {}
	r.HandleFunc("/", handleRequest)
}

@ramya-rao-a
Copy link
Contributor

@doxxx Wont there be a genuine case of when you do want to pass the result of a function as a parameter to another function?

@doxxx
Copy link
Contributor

doxxx commented Jul 19, 2018

Quite possibly. I was hoping this could be smart enough to determine that by expected type signature.

Perhaps I should just disable this option in the extension. The popup help for function parameters is fairly reliable now, so for me there's less need for expanding the parameter list into the file.

@ramya-rao-a
Copy link
Contributor

I was hoping this could be smart enough to determine that by expected type signature.

Since the extension itself isnt aware of the Go AST, we relied on very basic things like "look for , or )". So, not the code is not smart enough. I wouldnt want to add AST awareness to the code just for this feature.

What I can do is to wait for more feedback. If more users find this feature buggy, then we can revert it.

@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants