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

Show completion for newly created package. #1191

Merged
merged 42 commits into from
Sep 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
6d098d6
Refactor getNonVendorPackages to getPackages and respect the 1.9 vers…
uudashr Aug 31, 2017
ab7465f
Inline the function test arguments building
uudashr Aug 31, 2017
a500a42
Test all packages for Go 1.9 use "./...", the lower still remain usin…
uudashr Aug 31, 2017
ddc4d02
Merge remote-tracking branch 'upstream/master' into faster-go-list-all
uudashr Sep 4, 2017
730874a
Fix autocomplete un-imported packages: show newly added package
uudashr Sep 4, 2017
e265725
Use spawn to avoid maxBuffer exceeded
uudashr Sep 4, 2017
75c19a3
Support large output by using spawn
uudashr Sep 4, 2017
9dadbca
Completions shows result instantly, no longer need to pre-call to goL…
uudashr Sep 4, 2017
1a4c748
Fix lint warning
uudashr Sep 4, 2017
f7a75d0
Expect proper gopkgs binary
uudashr Sep 4, 2017
c7e0f4f
Change the gopkgs package
uudashr Sep 4, 2017
9583a0b
Use spawn instead of execFile to support large output
uudashr Sep 4, 2017
6ab865b
Use gopkgs from http://github.com/uudashr/gopkgs
uudashr Sep 7, 2017
41b1fa5
Merge branch 'master' into faster-go-list-all
uudashr Sep 7, 2017
40b2efd
Update the go tools in .travis.yml
uudashr Sep 7, 2017
95638f7
Fix the gopkgs missing tools detection
uudashr Sep 7, 2017
805ac03
Cache the gopkgs tool invocation result
uudashr Sep 7, 2017
6febef2
Refresh gopkgs cache
uudashr Sep 7, 2017
a2a1334
Drop the cache for gopkgs since it already fast enough
uudashr Sep 7, 2017
93379bc
Adapt with the changes of gopkgs by using -format
uudashr Sep 7, 2017
fe23338
trigger travis build: use array desctructuring assignment
uudashr Sep 7, 2017
c60374a
Install libsecret-1-0
uudashr Sep 9, 2017
780b744
Fix the travis script
uudashr Sep 9, 2017
d36aaeb
Fix the travis script, install libsecret-1-0
uudashr Sep 9, 2017
085d48f
Use sudo apt-get install
uudashr Sep 9, 2017
e1faec1
apt-get should buy some time
uudashr Sep 10, 2017
2c273e6
Add go 1.9.x to the .travis.yml
uudashr Sep 11, 2017
7d91dd8
Prompt for missing tools when "gopkgs" not found
uudashr Sep 11, 2017
6533a93
Update the comment of goListAll
uudashr Sep 11, 2017
8fb142e
Revert back the function name "goPackages" to "goNonVendorPackages"
uudashr Sep 11, 2017
ffcb0ef
Rename "goListAll" to "getAllPackages"
uudashr Sep 11, 2017
819c3db
Use existing getImportablePackages
uudashr Sep 11, 2017
636efa5
Missing tool prompt will be handled by function caller
uudashr Sep 11, 2017
f57805c
Remove unused import
uudashr Sep 11, 2017
e6a39ae
Handle missing tool when browsing package
uudashr Sep 11, 2017
a62c95c
Use cache if still used for less than 5s
uudashr Sep 11, 2017
e6f9454
Ignore the stderr to avoid stuck
uudashr Sep 11, 2017
e2ef816
Fix lint warning
uudashr Sep 11, 2017
2119ec1
Prompt to update gopkgs if using old pkgs
ramya-rao-a Sep 12, 2017
18cbcf8
Cleanup
ramya-rao-a Sep 12, 2017
2a898f0
Fix tests
ramya-rao-a Sep 12, 2017
7a5c9db
Sort pkgs before testing
ramya-rao-a Sep 12, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go:
- 1.6.x
- 1.7.x
- 1.8.x
- 1.9.x
- tip

sudo: false
Expand All @@ -17,7 +18,7 @@ before_install:
- if [ $TRAVIS_OS_NAME == "linux" ]; then
export CXX="g++-4.9" CC="gcc-4.9" DISPLAY=:99.0;
sh -e /etc/init.d/xvfb start;
sleep 3;
sudo apt-get update && sudo apt-get install -y libsecret-1-0;
fi

install:
Expand All @@ -41,7 +42,7 @@ install:
- go get -u -v github.com/ramya-rao-a/go-outline
- go get -u -v sourcegraph.com/sqs/goreturns
- go get -u -v golang.org/x/tools/cmd/gorename
- go get -u -v github.com/tpng/gopkgs
- go get -u -v github.com/uudashr/gopkgs/cmd/gopkgs
- go get -u -v github.com/acroca/go-symbols
- go get -u -v github.com/alecthomas/gometalinter
- go get -u -v github.com/cweill/gotests/...
Expand Down
16 changes: 4 additions & 12 deletions src/goBrowsePackage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import vscode = require('vscode');
import cp = require('child_process');
import { getGoRuntimePath } from './goPath';
import path = require('path');
import { goListAll, isGoListComplete } from './goPackages';
import { getAllPackages } from './goPackages';

export function browsePackages() {
let selectedText = '';
Expand Down Expand Up @@ -70,30 +70,22 @@ function showPackageFiles(pkg: string, showAllPkgsIfPkgNotFound: boolean) {
}

function showPackageList() {
if (!isGoListComplete()) {
return showTryAgainLater();
}

goListAll().then(pkgMap => {
getAllPackages().then(pkgMap => {
const pkgs: string[] = Array.from(pkgMap.keys());
if (!pkgs || pkgs.length === 0) {
if (pkgs.length === 0) {
return vscode.window.showErrorMessage('Could not find packages. Ensure `go list all` runs successfully.');
}

vscode
.window
.showQuickPick(pkgs, { placeHolder: 'Select a package to browse' })
.showQuickPick(pkgs.sort(), { placeHolder: 'Select a package to browse' })
.then(pkgFromDropdown => {
if (!pkgFromDropdown) return;
showPackageFiles(pkgFromDropdown, false);
});
});
}

function showTryAgainLater() {
vscode.window.showInformationMessage('Finding packages... Try after sometime.');
}

function getImportPath(text: string): string {
// Catch cases like `import alias "importpath"` and `import "importpath"`
let singleLineImportMatches = text.match(/^\s*import\s+([a-z,A-Z,_,\.]\w*\s+)?\"([^\"]+)\"/);
Expand Down
52 changes: 7 additions & 45 deletions src/goImport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,57 +11,19 @@ import { parseFilePrelude, isVendorSupported, getBinPath, getCurrentGoPath, getT
import { documentSymbols } from './goOutline';
import { promptForMissingTool } from './goInstallTools';
import path = require('path');
import { getRelativePackagePath } from './goPackages';
import { getCurrentGoWorkspaceFromGOPATH } from './goPath';
import { getImportablePackages } from './goPackages';

const missingToolMsg = 'Missing tool: ';

export function listPackages(excludeImportedPkgs: boolean = false): Thenable<string[]> {
let importsPromise = excludeImportedPkgs && vscode.window.activeTextEditor ? getImports(vscode.window.activeTextEditor.document) : Promise.resolve([]);
let vendorSupportPromise = isVendorSupported();
let goPkgsPromise = new Promise<string[]>((resolve, reject) => {
cp.execFile(getBinPath('gopkgs'), [], {env: getToolsEnvVars()}, (err, stdout, stderr) => {
if (err && (<any>err).code === 'ENOENT') {
return reject(missingToolMsg + 'gopkgs');
}
let lines = stdout.toString().split('\n');
if (lines[lines.length - 1] === '') {
// Drop the empty entry from the final '\n'
lines.pop();
}
return resolve(lines);
});
});

return vendorSupportPromise.then((vendorSupport: boolean) => {
return Promise.all<string[]>([goPkgsPromise, importsPromise]).then(values => {
let pkgs = values[0];
let importedPkgs = values[1];

if (!vendorSupport) {
if (importedPkgs.length > 0) {
pkgs = pkgs.filter(element => {
return importedPkgs.indexOf(element) === -1;
});
}
return pkgs.sort();
}
let pkgsPromise = getImportablePackages(vscode.window.activeTextEditor.document.fileName);

let currentFileDirPath = path.dirname(vscode.window.activeTextEditor.document.fileName);
let currentWorkspace = getCurrentGoWorkspaceFromGOPATH(getCurrentGoPath(), currentFileDirPath);
let pkgSet = new Set<string>();
pkgs.forEach(pkg => {
if (!pkg || importedPkgs.indexOf(pkg) > -1) {
return;
}
let relativePkgPath = getRelativePackagePath(currentFileDirPath, currentWorkspace, pkg);
if (relativePkgPath) {
pkgSet.add(relativePkgPath);
}
});

return Array.from(pkgSet).sort();
return Promise.all([pkgsPromise, importsPromise]).then(([pkgMap, importedPkgs]) => {
importedPkgs.forEach(pkg => {
pkgMap.delete(pkg);
});
return Array.from(pkgMap.keys()).sort();
});
}

Expand Down Expand Up @@ -133,4 +95,4 @@ export function addImport(arg: string) {
});
}
});
}
}
2 changes: 1 addition & 1 deletion src/goInstallTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function getTools(goVersion: SemVersion): { [key: string]: string } {
let goConfig = vscode.workspace.getConfiguration('go');
let tools: { [key: string]: string } = {
'gocode': 'github.com/nsf/gocode',
'gopkgs': 'github.com/tpng/gopkgs',
'gopkgs': 'github.com/uudashr/gopkgs/cmd/gopkgs',
'go-outline': 'github.com/ramya-rao-a/go-outline',
'go-symbols': 'github.com/acroca/go-symbols',
'guru': 'golang.org/x/tools/cmd/guru',
Expand Down
4 changes: 1 addition & 3 deletions src/goMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import { addTags, removeTags } from './goModifytags';
import { parseLiveFile } from './goLiveErrors';
import { GoCodeLensProvider } from './goCodelens';
import { implCursor } from './goImpl';
import { goListAll } from './goPackages';
import { browsePackages } from './goBrowsePackage';

export let errorDiagnosticCollection: vscode.DiagnosticCollection;
Expand Down Expand Up @@ -66,7 +65,6 @@ export function activate(ctx: vscode.ExtensionContext): void {
}
}
});
goListAll();
offerToInstallTools();
let langServerAvailable = checkLanguageServer();
if (langServerAvailable) {
Expand Down Expand Up @@ -421,4 +419,4 @@ function didLangServerConfigChange(useLangServer: boolean, langServerFlags: stri
}
}
return false;
}
}
103 changes: 58 additions & 45 deletions src/goPackages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,54 +2,60 @@ import vscode = require('vscode');
import cp = require('child_process');
import path = require('path');
import { getGoRuntimePath, getCurrentGoWorkspaceFromGOPATH } from './goPath';
import { isVendorSupported, getCurrentGoPath, getToolsEnvVars } from './util';
import { isVendorSupported, getCurrentGoPath, getToolsEnvVars, getGoVersion, getBinPath, SemVersion } from './util';
import { promptForMissingTool, promptForUpdatingTool } from './goInstallTools';

let allPkgs = new Map<string, string>();
let goListAllCompleted: boolean = false;
let goListAllPromise: Promise<Map<string, string>>;
let allPkgsCache: Map<string, string>;
let allPkgsLastHit: number;

export function isGoListComplete(): boolean {
return goListAllCompleted;
}

/**
* Runs go list all
* @returns Map<string, string> mapping between package import path and package name
*/
export function goListAll(): Promise<Map<string, string>> {
let goRuntimePath = getGoRuntimePath();

if (!goRuntimePath) {
vscode.window.showInformationMessage('Cannot find "go" binary. Update PATH or GOROOT appropriately');
return Promise.resolve(null);
}

if (goListAllPromise) {
return goListAllPromise;
}
function getAllPackagesNoCache(): Promise<Map<string, string>> {
return new Promise<Map<string, string>>((resolve, reject) => {
const cmd = cp.spawn(getBinPath('gopkgs'), ['-format', '{{.Name}};{{.ImportPath}}'], { env: getToolsEnvVars() });
const chunks = [];
const errchunks = [];
let err: any;
cmd.stdout.on('data', d => chunks.push(d));
cmd.stderr.on('data', d => errchunks.push(d));
cmd.on('error', e => err = e);
cmd.on('close', () => {
let pkgs = new Map<string, string>();
if (err && err.code === 'ENOENT') {
return promptForMissingTool('gopkgs');
}

goListAllPromise = new Promise<Map<string, string>>((resolve, reject) => {
// Use `{env: {}}` to make the execution faster. Include GOPATH to account if custom work space exists.
const env: any = getToolsEnvVars();
if (err || errchunks.length > 0) return resolve(pkgs);

const cmd = cp.spawn(goRuntimePath, ['list', '-f', '{{.Name}};{{.ImportPath}}', 'all'], { env: env, stdio: ['pipe', 'pipe', 'ignore'] });
const chunks = [];
cmd.stdout.on('data', (d) => {
chunks.push(d);
});
const output = chunks.join('');
if (output.indexOf(';') === -1) {
// User might be using the old gopkgs tool, prompt to update
return promptForUpdatingTool('gopkgs');
}

cmd.on('close', (status) => {
chunks.join('').split('\n').forEach(pkgDetail => {
output.split('\n').forEach((pkgDetail) => {
if (!pkgDetail || !pkgDetail.trim() || pkgDetail.indexOf(';') === -1) return;
let [pkgName, pkgPath] = pkgDetail.trim().split(';');
allPkgs.set(pkgPath, pkgName);
pkgs.set(pkgPath, pkgName);
});
goListAllCompleted = true;
return resolve(allPkgs);
return resolve(pkgs);
});
});
}

/**
* Runs gopkgs
* @returns Map<string, string> mapping between package import path and package name
*/
export function getAllPackages(): Promise<Map<string, string>> {
let useCache = allPkgsCache && allPkgsLastHit && (new Date().getTime() - allPkgsLastHit) < 5000;
if (useCache) {
allPkgsLastHit = new Date().getTime();
return Promise.resolve(allPkgsCache);
}

return goListAllPromise;
return getAllPackagesNoCache().then((pkgs) => {
allPkgsLastHit = new Date().getTime();
return allPkgsCache = pkgs;
});
}

/**
Expand All @@ -59,13 +65,14 @@ export function goListAll(): Promise<Map<string, string>> {
*/
export function getImportablePackages(filePath: string): Promise<Map<string, string>> {

return Promise.all([isVendorSupported(), goListAll()]).then(values => {
return Promise.all([isVendorSupported(), getAllPackages()]).then(values => {
let isVendorSupported = values[0];
let pkgs = values[1];
let currentFileDirPath = path.dirname(filePath);
let currentWorkspace = getCurrentGoWorkspaceFromGOPATH(getCurrentGoPath(), currentFileDirPath);
let pkgMap = new Map<string, string>();

allPkgs.forEach((pkgName, pkgPath) => {
pkgs.forEach((pkgName, pkgPath) => {
if (pkgName === 'main') {
return;
}
Expand All @@ -87,7 +94,7 @@ export function getImportablePackages(filePath: string): Promise<Map<string, str
* If given pkgPath is not vendor pkg, then the same pkgPath is returned
* Else, the import path for the vendor pkg relative to given filePath is returned.
*/
export function getRelativePackagePath(currentFileDirPath: string, currentWorkspace: string, pkgPath: string): string {
function getRelativePackagePath(currentFileDirPath: string, currentWorkspace: string, pkgPath: string): string {
let magicVendorString = '/vendor/';
let vendorIndex = pkgPath.indexOf(magicVendorString);
if (vendorIndex === -1) {
Expand All @@ -113,7 +120,7 @@ export function getRelativePackagePath(currentFileDirPath: string, currentWorksp
}

/**
* Returns import paths for all non vendor packages under given folder
* Returns import paths for all packages under given folder (vendor will be excluded)
*/
export function getNonVendorPackages(folderPath: string): Promise<string[]> {
let goRuntimePath = getGoRuntimePath();
Expand All @@ -123,15 +130,21 @@ export function getNonVendorPackages(folderPath: string): Promise<string[]> {
return Promise.resolve(null);
}
return new Promise<string[]>((resolve, reject) => {
const childProcess = cp.spawn(goRuntimePath, ['list', './...'], { cwd: folderPath, env: getToolsEnvVars() });
const chunks = [];
let childProcess = cp.spawn(goRuntimePath, ['list', './...'], { cwd: folderPath, env: getToolsEnvVars() });
let chunks = [];
childProcess.stdout.on('data', (stdout) => {
chunks.push(stdout);
});

childProcess.on('close', (status) => {
const pkgs = chunks.join('').toString().split('\n').filter(pkgPath => pkgPath && !pkgPath.includes('/vendor/'));
return resolve(pkgs);
let pkgs = chunks.join('').toString().split('\n');
getGoVersion().then((ver: SemVersion) => {
if (ver && (ver.major > 1 || (ver.major === 1 && ver.minor >= 9))) {
resolve(pkgs);
} else {
resolve(pkgs.filter(pkgPath => pkgPath && !pkgPath.includes('/vendor/')));
}
});
});
});
}
Expand Down
14 changes: 6 additions & 8 deletions src/goSuggest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ interface GoCodeSuggestion {

export class GoCompletionItemProvider implements vscode.CompletionItemProvider {

private gocodeConfigurationComplete = false;
private pkgsList = new Map<string, string>();

public provideCompletionItems(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): Thenable<vscode.CompletionItem[]> {
Expand Down Expand Up @@ -215,16 +214,12 @@ export class GoCompletionItemProvider implements vscode.CompletionItemProvider {
}
// TODO: Shouldn't lib-path also be set?
private ensureGoCodeConfigured(): Thenable<void> {
getImportablePackages(vscode.window.activeTextEditor.document.fileName).then(pkgMap => {
let setPkgsList = getImportablePackages(vscode.window.activeTextEditor.document.fileName).then(pkgMap => {
this.pkgsList = pkgMap;
return;
});

return new Promise<void>((resolve, reject) => {
// TODO: Since the gocode daemon is shared amongst clients, shouldn't settings be
// adjusted per-invocation to avoid conflicts from other gocode-using programs?
if (this.gocodeConfigurationComplete) {
return resolve();
}
let setGocodeProps = new Promise<void>((resolve, reject) => {
let gocode = getBinPath('gocode');
let autobuild = vscode.workspace.getConfiguration('go')['gocodeAutoBuild'];
let env = getToolsEnvVars();
Expand All @@ -235,6 +230,9 @@ export class GoCompletionItemProvider implements vscode.CompletionItemProvider {
});
});

return Promise.all([setPkgsList, setGocodeProps]).then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the -format option I am guessing that gopkgs is a bit slower than when you made the comparison between go list all and gopkgs in the description of this PR.

What are your current numbers?

Every new word typed triggers completion. So you can imagine that gopkgs will be run for every word typed.
Some users have a whole lot of packages that go list all took over 10 seconds to finish. How does gopkgs -format fare?

I still believe we should use cache and have the cache updated at a regular interval instead of running gopkgs for every word.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gpkgs -format faster than go list all

For 3382 packages packages:

go list all  4.62s user 3.27s system 124% cpu 6.360 total
gopkgs  0.65s user 0.70s system 264% cpu 0.508 total

6.36s vs 0.5s, big difference.
This gopkgs using fastwalk, faster than golang standard fs.Walk().

Not every word typed, it only invoked on only when completion triggered. Typing "strings" 7 words typed will only cost single gopkgs invocation. So this is quite cheap and fast enough.

Cache will be much better of course.
If we cache for 10 or 20 second, user probably can create new file in new folder with new package faster than cache expiry time. So new package will not provided by completion. The workaround crossed in my mind is we need to some kind of file system watcher and check if there is changed file, then invalidate the cache or run gopkgs immediately to refresh the cache, etc. My suggestion is we can do this cache things on another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed my mind... let's try to use 5s cache expiry

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not every word typed, it only invoked on only when completion triggered.

Completion is triggered for every word typed. I think you misunderstood my statement as every "letter" typed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh... yup..

return;
});
}

// Return importable packages that match given word as Completion Items
Expand Down
Loading