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

Add Package/Interface Dropdowns to Generate Interface Stubs Command #1547

Open
madxmike opened this issue Jun 6, 2021 · 15 comments · May be fixed by #1592
Open

Add Package/Interface Dropdowns to Generate Interface Stubs Command #1547

madxmike opened this issue Jun 6, 2021 · 15 comments · May be fixed by #1592
Labels
FeatureRequest HelpWanted Issues that are not prioritized by the maintainers. Help is requested from community contributors. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@madxmike
Copy link

madxmike commented Jun 6, 2021

Is your feature request related to a problem? Please describe.
One of the features that I use the most in this extension is the "Generate Interface Stubs" command. It's great, but one of my main gripes with the feature is having to actually remember both the package and name of the interface I am wanting to implement. Often I'll forget mid-typing, quit the prompt, go reference what I am trying to implement, return to my file, and then do the command again. Even more often I'll just mistype and nothing will be generated.

Describe the solution you'd like
It would be great to have a drop down list appear when you get to the interface name of the command. For instance:

When you type f *File and then space a drop down will appear with a list of packages, much like the list found in the "Add Import" command. You can then type the package name or choose from the list. After the dot, a list of all the interfaces found in that package will appear. You can then type the interface name or choose from the list.

Describe alternatives you've considered
Another idea would just be to check if the interface exists after every keystroke, and then give a warning if it does not. I think that this would feel bad since the extension would flash a warning to you every time you type.

Additional context
Example package selection
Example package selection
Example interface selection
Example interface selection

@gopherbot gopherbot added this to the Untriaged milestone Jun 6, 2021
@findleyr findleyr modified the milestones: Untriaged, Backlog Jun 7, 2021
@findleyr
Copy link
Contributor

findleyr commented Jun 7, 2021

Hi, thanks for the suggestion.

This makes sense, though unlikely to be prioritized soon (the team is busy working on debugging and gopls). Looking at the available APIs I think we'd have to switch from an inputBox to quickPick, but it could be doable.

CC @hyangah @suzmue for their thoughts. Perhaps this could be an opportunity for external contribution.

@hyangah hyangah added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 8, 2021
@hyangah
Copy link
Contributor

hyangah commented Jun 8, 2021

That's a bit unusual use case of quickPick or inputBox. I am guessing it requires some creativity on top of what vscode api offers by default. If any of you know reference or example to combine quickPick/inputBox + completion, please update this issue. Alternative is to have separate quickPicks for the first (the receiver type) and the second (the interface type) args.

Another question is where to get the candidate interface list. Obvious choice is gopls. In Go, a type can implement any interface type without explicitly importing the package where the interface is defined. In theory, the list can be infinite but we will need to come up with a reasonable heuristic.

@findleyr
Copy link
Contributor

findleyr commented Jun 8, 2021

@hyangah FWIW my comment was based on microsoft/vscode#426.

Gopls could definitely provide the candidate list. I think the hardest part of implementing this would be figuring out if the desired UI is possible.

@findleyr
Copy link
Contributor

This feature could be useful, but we won't have time to work on it anytime soon. It mostly needs some UI research to see if there is a clean way to implement completion callbacks for dialog boxes. If someone could figure that out, I could add a custom command to gopls to populate the completion data.

Marking this as help-wanted as the UI research should be mostly self-contained.

@findleyr findleyr added the HelpWanted Issues that are not prioritized by the maintainers. Help is requested from community contributors. label Jun 11, 2021
@day-dreams
Copy link

day-dreams commented Jun 24, 2021

https://marketplace.visualstudio.com/items?itemName=kakaxi.autoimpl

i just implement an extension for myself. hope it is useful for you too.
usage

@hyangah
Copy link
Contributor

hyangah commented Jun 24, 2021

@day-dreams The use of snippet (and placeholders) is smart and seems to work pretty nice. Do you mind contributing to this extension? It looks like the existing code is in https://github.com/golang/vscode-go/blob/master/src/goImpl.ts - and from the look this extension-specific logic is only to check whether impl is installed or not.

@day-dreams
Copy link

day-dreams commented Jun 24, 2021

@hyangah of course, i would like to help imporve this extesion . gonna make a pr request soon

day-dreams added a commit to day-dreams/vscode-go that referenced this issue Jun 25, 2021
* change InputBox to QuickPick
* search workspace interface symbol according to keyword
* change static code text to code snippet
@day-dreams
Copy link

@day-dreams The use of snippet (and placeholders) is smart and seems to work pretty nice. Do you mind contributing to this extension? It looks like the existing code is in https://github.com/golang/vscode-go/blob/master/src/goImpl.ts - and from the look this extension-specific logic is only to check whether impl is installed or not.

made a pr request, hope to be merged. #1592

@madxmike
Copy link
Author

Wow that looks great! I love that it does a multi-cursor edit for the receiver name. Seems much more intuitive and smooth than the current ui.

day-dreams pushed a commit to day-dreams/vscode-go that referenced this issue Jun 26, 2021
* change InputBox to QuickPick
* search workspace interface symbol according to keyword
* change static code text to code snippet

For golang#1547
day-dreams added a commit to day-dreams/vscode-go that referenced this issue Jun 26, 2021
* change InputBox to QuickPick
* search workspace interface symbol according to keyword
* change static code text to code snippet

For golang#1547
@hyangah hyangah linked a pull request Jun 30, 2021 that will close this issue
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/330850 mentions this issue: src/goImpl: improve goimpl implememtations (#1547)

@hyangah
Copy link
Contributor

hyangah commented Jun 30, 2021

@day-dreams Thanks for the PR! I was playing with it and it seems like we need to sort out a couple of issues related to the use of the workspace symbol provider currently.

  • workspace symbol results from gopls currently include ambiguous results - see x/tools/gopls: spurious workspace symbol items go#46997

  • the PR first retrieves the workspace symbol list and then filters only "Interface" type symbols out of the list. Since gopls returns only the first K matching items, that means, after filtering, it's possible only a small number or even zero of Interface types are left - which may not include what users want. I think a better strategy is to implement a specialized command on the language server side, which returns only the interface types. That way, it may be more efficient and may include the interesting interface types in the list.
    cc gopls-devs for advice: @stamblerre @findleyr

  • we may need a fallback mechanism in case the candidate list has nothing matching - maybe a fake item that triggers the old logic. That can be done in a separate, follow up change though if it becomes a problem for many users.

@day-dreams
Copy link

day-dreams commented Jul 1, 2021

@day-dreams Thanks for the PR! I was playing with it and it seems like we need to sort out a couple of issues related to the use of the workspace symbol provider currently.

  • workspace symbol results from gopls currently include ambiguous results - see x/tools/gopls: spurious workspace symbol items go#46997
  • the PR first retrieves the workspace symbol list and then filters only "Interface" type symbols out of the list. Since gopls returns only the first K matching items, that means, after filtering, it's possible only a small number or even zero of Interface types are left - which may not include what users want. I think a better strategy is to implement a specialized command on the language server side, which returns only the interface types. That way, it may be more efficient and may include the interesting interface types in the list.
    cc gopls-devs for advice: @stamblerre @findleyr
  • we may need a fallback mechanism in case the candidate list has nothing matching - maybe a fake item that triggers the old logic. That can be done in a separate, follow up change though if it becomes a problem for many users.

it seems this feature needs more internal supports。 before vscode-go implements this feature, i will continue to use my extension. https://marketplace.visualstudio.com/items?itemName=kakaxi.autoimpl

hope you can support this soon :)

@madxmike
Copy link
Author

Now that x/tools/gopls: spurious workspace symbol items go#46997 has been merged, is there any update on the status of this? This is still something I would really love to see as baseline in the extension,

@hyangah hyangah modified the milestones: Backlog, v0.28.0 Aug 3, 2021
@hyangah hyangah modified the milestones: v0.28.0, v0.29.0 Sep 16, 2021
@hyangah hyangah removed this from the v0.29.0 milestone Oct 26, 2021
@hyangah hyangah added this to the v0.30.0 milestone Oct 26, 2021
@hyangah hyangah modified the milestones: v0.30.0, On Deck Dec 9, 2021
@hyangah hyangah modified the milestones: On Deck, v0.31.0 Jan 5, 2022
@hyangah
Copy link
Contributor

hyangah commented Jan 14, 2022

@day-dreams Can you please take a look at the diff I made from your PR so we can include it in the next release? Thanks again!

https://go-review.googlesource.com/c/vscode-go/+/330850/4..7/src/goImpl.ts

@madxmike
Copy link
Author

madxmike commented Jun 6, 2022

I finally got around to working in go again, and decided to try and finish this feature.

I had saw the work done in golang/go#37537 a few weeks ago. I think it would be best to leverage that work. I'm not sure we can utilize the snippet approach demonstrated above with the current gopls work since we need the concrete type info.

I made a quick poc for the picker using my original idea above to grab both the concrete and interface type's symbol info. I'm working on integrating a gopls command into the work above, but just wanted to show a quick gif of the picker for thoughts.

interface_stub.mp4

One thing the gif doesnt show is that the struct quick pick is prefilled with the cursor's current selection if its available. I played around with it and it felt natural to me, but I'm not sure if everyone would like that functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest HelpWanted Issues that are not prioritized by the maintainers. Help is requested from community contributors. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants