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

Fix gopath bin location search and error in error line parsing in windows #46

Merged
merged 2 commits into from
Nov 20, 2015
Merged

Fix gopath bin location search and error in error line parsing in windows #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a28cc4
Correct the use of bin path in windows
pcmind Nov 20, 2015
821b545
Corrected line matching in error handling for go build tool
pcmind Nov 20, 2015
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
4 changes: 2 additions & 2 deletions src/goCheck.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ export function check(filename: string, buildOnSave = true, lintOnSave = true, v
var lines = stderr.toString().split('\n');
var ret: ICheckResult[] = [];
for(var i = 1; i < lines.length; i++) {
var match = /(.*):(\d+): (.*)/.exec(lines[i]);
var match = /([^:]*):(\d+)(:\d+)?: (.*)/.exec(lines[i]);
if(!match) continue;
var [_, file, lineStr, msg] = match;
var [_, file, lineStr, charStr, msg] = match;
var line = +lineStr;
ret.push({ file: path.resolve(cwd, file), line, msg, severity: "error" });
}
Expand Down
12 changes: 9 additions & 3 deletions src/goPath.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { showGoStatus, hideGoStatus } from './goStatus'
var binPathCache : { [bin: string]: string;} = {}
Copy link

Choose a reason for hiding this comment

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

binPathCache is not used?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed - looks like we've never been writing to the cache. We should either do that or remove the cache code for now pending figuring out how much of a perf hit this codepath is.


export function getBinPath(binname) {
binname = correctBinname(binname)
Copy link

Choose a reason for hiding this comment

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

Correcting should be done after checking cache.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed.

if(binPathCache[binname]) return binPathCache[binname];
var workspaces = getGOPATHWorkspaces();
var binpath: string;
Expand All @@ -27,6 +28,13 @@ export function getBinPath(binname) {
return path.join(process.env["GOPATH"], "bin", binname);
Copy link

Choose a reason for hiding this comment

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

workspaces will always be of .length > 0 and if no bin is found returning ws1:ws2/bin/name makes no sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

What would you suggest instead? Perhaps just return binname and rely on finding it in PATH?

Edit: Actually that won't work by itself since we use the returned path in calls to fs.existsSync.

Copy link

Choose a reason for hiding this comment

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

Leaving it on PATH would make sense.

}

function correctBinname(binname) {
if (process.platform === 'win32')
return binname + ".exe";
else
return binname
}

function getGOPATHWorkspaces() {
var seperator : string;
switch(os.platform()) {
Expand Down Expand Up @@ -75,9 +83,7 @@ export function setupGoPathAndOfferToInstallTools() {
}
var keys = Object.keys(tools)
Promise.all(keys.map(tool => new Promise<string>((resolve, reject) => {
let toolPath = path.join(process.env["GOPATH"], 'bin', tool);
if (process.platform === 'win32')
toolPath = toolPath + ".exe";
let toolPath = getBinPath(tool);
fs.exists(toolPath, exists => {
resolve(exists ? null : tools[tool])
});
Expand Down