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

[improvement] Completion mess with import or already set variables #598

Closed
bennyscetbun opened this issue Nov 3, 2016 · 10 comments
Closed
Assignees

Comments

@bennyscetbun
Copy link

Hi, there is a new completion stuff that is pretty great.
It can let you choose a package from a list of all available package even if not imported yet.
Pretty good.
But if i have a variable with the name of a package, the completion start with unimported package when i want to use the variable.
Also if you ve already imported a package with that name some wierd stuff happen and could lead to a file with multiple import of same package but with different path.
Solution:
If a variable/package is already set in the current scope dont show any package that can be imported with the same name and also sort all the result the way that every variables already set in the scope should be first in the completion choice

@bennyscetbun bennyscetbun changed the title Completion mess with import [improvement] Completion mess with import or already set variables Nov 3, 2016
@ramya-rao-a
Copy link
Contributor

Hi @bennyscetbun

I was trying to repro the case when a variable and unimported package have the same name

image

For me, the variable comes first and then the package. Can you try the code from above and see what you get? And maybe share a case where the package comes before the variable?

Also can you elaborate on the below with an example?

Also if you ve already imported a package with that name some wierd stuff happen and could lead to a file with multiple import of same package but with different path.

@bennyscetbun
Copy link
Author

bennyscetbun commented Nov 5, 2016

Hi
This is an example where it fails
screen shot 2016-11-05 at 02 56 12

and for example if you ve got the same package in a lot of different places and already imported it
screen shot 2016-11-05 at 02 59 15

And about :

Also if you ve already imported a package with that name some wierd stuff happen and could lead to a file with multiple import of same package but with different path.

this what happens if i press enter at the last screen shot
screen shot 2016-11-05 at 03 03 21

I hope it helps :)

@bennyscetbun
Copy link
Author

Btw you should avoid to show path of packages with vendor in it ???

@ramya-rao-a
Copy link
Contributor

#603 will group all the packages and show them in the end after the variables/functions/others. You will get that in the next update

About multiple packages having the same name, I can see how in your case it could get annoying. But, wouldn't there be a valid case where you would have 2 packages with same name but offer different things. Why shouldn't auto-complete suggest them then?

About packages with "vendor" in it. If it is a vendor package in the current Go project, then the path will not have "vendor" in it. In the above case, the package is a vendor from another project. Technically, you can still import it and use it....

@bennyscetbun
Copy link
Author

Tell me if i am wrong but if you import a package in a vendor of an other project you will get an error.
For example i am in github.com/bennyscetbun/testgo project and imported a vendored package.
screen shot 2016-11-05 at 11 01 43

@ramya-rao-a
Copy link
Contributor

That's interesting, I tried something similar and didn't get any build errors (in the output window with buildOnSave set to true), which is why I did not remove the vendored packages from the list of importable packages.

When I build my code independently outside of VS Code, I get a different error
imports github.com/godoctor/godoctor/vendor/github.com/willf/bitset: must be imported as github.com/willf/bitset

But you are right. The functions disallowVendor and disallowVendorVisibility in https://golang.org/src/cmd/go/pkg.go make it clear, the importing a vendor package from another project is not allowed.

Will make that change.

@bennyscetbun
Copy link
Author

bennyscetbun commented Nov 6, 2016

Hi @ramya-rao-a
i have one tiny last thing about:

About multiple packages having the same name, I can see how in your case it could get annoying. But, wouldn't there be a valid case where you would have 2 packages with same name but offer different things. Why shouldn't auto-complete suggest them then?

You wont be able to build with 2 packages with the same name so importing them in the middle of your code wont be usefull.
The only way to add a package with the same name is to force the name at import and you ve already done the completion when doing import.
From my PoV not showing other packages that you wont be able to import or that wont override a scope variable is a good idea :)

And Thanks again for your hard work on this. vscode-go rox :)

@ramya-rao-a
Copy link
Contributor

I knew about the error on using 2 pkgs with same name. I just expected the user to add an alias to one of them 😊

What can be done in such cases is for the auto complete itself to add an alias. That should be easy.

And thanks for all the feedback! That's how we can together make this a great product 😊

@ramya-rao-a
Copy link
Contributor

On second thoughts, I'll go with your suggestion of not showing the importable package if there is an identifier with the same name already in the Go file

@ramya-rao-a
Copy link
Contributor

The latest update (0.6.48) has all the fixes discussed here.
Happy Coding!

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

No branches or pull requests

2 participants