Prioritize current project import suggestions - #1782
Conversation
| private getMatchingPackages(word: string, suggestionSet: Set<string>): vscode.CompletionItem[] { | ||
| if (!word) return []; | ||
| let completionItems = []; | ||
| const rootPath = vscode.workspace.rootPath.slice(getCurrentGoPath().length + 5); |
There was a problem hiding this comment.
This will fail in a few cases
- Multi-root mode. To fix this you can use
vscode.workspace.getWorkspaceFolder(fileUri) - The file being edited is not part of the folder opened in VS Code or the file alone has been opened in VS Code. No folders. In this case
vscode.workspace.getWorkspaceFolder(fileUri)would not return a workspace, so you can fall back to the parent folder of the file. - The file being edited is not part of the gopath
- The
getCurrentGopathreturns multiple gopaths likeprojectA:projectBorprojectA;projectB. To solve this you can usegetCurrentGoWorkspaceFromGOPATHmethod fromgoPath.ts
| item.sortText = isStandardPackage ? 'za' : 'zb'; | ||
| completionItems.push(item); | ||
| if (pkgPath.startsWith(rootPath)) { | ||
| item.sortText = 'a'; |
There was a problem hiding this comment.
If all the packages share the same sortText, I believe that the sort order then is the order that they appear in the completionItems array. Then why change the sortText here?
There was a problem hiding this comment.
Ramya Rao (@ramya-rao-a) this is so that the unimported packages from the same project (or workspace) are prioritized over the rest of the package suggestions. Say you are in project a and you start typing b in main.go, when the imports are suggested i'd like to see a/b before someOtherProject/b so i can just hit enter and a/b is imported. Most of the time when you wanna import a package, it's most likely either from your own project, or from the standard library, or a third party. Third party libraries have special cool names so they usually come first anyway. But when you type generic package name like log or errors, you get too many suggestions from GOPATH and you most likely just want the log package from your own project.
There was a problem hiding this comment.
Take a hypothetical case
- lab, lag, large, lamb are standard pkgs
- github.com/mine/log, github.com/you/log are custom packages
The user is currently working on the github.com/mine/myproject and types l
The suggestions would now appear as
- log -> github.com/mine/log
- lab
- lag
- large
- lamb
- log -> github.com/you/log
This would be counter intutive, as all the std pkgs would be expected to be sorted above alphabetically.
The solution to your problem should be such that your package is sorted to the top but only among other packages with the same name. So taking the same example as above, typing l should result in
- lab
- lag
- large
- lamb
- log -> github.com/mine/log
- log -> github.com/you/log
There was a problem hiding this comment.
Ramya Rao (@ramya-rao-a) ah, yes that was intentional. If you have a log package inside your project, then you are most likely going to import your own log package than the standard library one.
But I'm still happy if the requirement is that standard packages are always shown first.
Which way do you prefer? And if the latter, how would you like me to make the sorting work? I'm thinking something like:
if (isStandardLibrary) sortLetter = 'a';
else if (isProjectLibrary) sortLetter = 'b';
would that work? assuming we have a function to figure out if an import is std lib :)
There was a problem hiding this comment.
My previous comment assumed that the unimported package suggestions were sorted among themselves alphabetically. Which is not the case. My bad.
How about item.sortText = isStandardPackage ? 'za' : pkgPath.startsWith(rootPath) ? 'zb' : 'zc'?
We need the z as we dont want any of the unimported packages to appear before other normal symbol suggestions.
6bc8fae to
dd218e2
Compare
|
Ramya Rao (@ramya-rao-a) i updated and commented the pr, thanks for taking a look! |
|
Ramya Rao (@ramya-rao-a) I've applied your last suggested edit, which seems to be working as intended. Thanks again for pointing out all the edge cases! Let me know if this needs further edits. |
Ramya Rao (ramya-rao-a)
left a comment
Ramya Rao (ramya-rao-a)
left a comment
There was a problem hiding this comment.
Marwan Sulaiman (@marwan-at-work) I pushed a commit with slight re-factoring. Can you test it from your end? Then, I'll merge the PR.
|
Ramya Rao (@ramya-rao-a) tested it out. Looks good. Thank you!! |
|
Ramya Rao (@ramya-rao-a) ok to merge on my end. Something kind of related but probably for another pr: would it be possible to just remove all packages nested under I'm happy to push another PR for it if you can give me some direction. Thanks! |
|
There is an ongoing discussion for vgo in #1532. Add you note there with some links to documentation and we can take it from there. |
|
This fix is now out in the latest update (0.6.85) to the Go extension. |
2 participants
This PR will make it so that "import suggestions" will always put your project's packages at the top. This really helps when there are very common package names like
logorerrorsand you want to import them through the autocomplete feature. Before this pr, you'd have to scroll through 10s if not 100s of packages to find the one form you're workspace.From the screenshot below, you can see that if you're inside
upspin.io's sourcode, then the autocomplete shows you imports from that package first.