Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-mccall committed May 2, 2020
1 parent 0091369 commit 47d650a
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 37 deletions.
64 changes: 39 additions & 25 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,16 @@
"abort-controller": "^3.0.0",
"jsonc-parser": "^2.1.0",
"node-fetch": "^2.6.0",
"readdirp": "^3.4.0",
"semver": "^7.3.2",
"unzipper": "^0.10.11",
"vscode-languageclient": "6.1.0",
"vscode-languageserver-types": "^3.15.1"
"vscode-languageserver-types": "^3.15.1",
"which": "^2.0.2"
},
"devDependencies": {
"@types/node-fetch": "^2.5.6",
"@types/unzipper": "^0.10.3",
"@types/yauzl": "^2.9.1",
"@types/glob": "^7.1.1",
"@types/mocha": "^7.0.2",
"@types/node": "^12.8.1",
Expand Down
4 changes: 2 additions & 2 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ export async function activate(context: vscode.ExtensionContext) {
try {
await util.promisify(which)(clangdPath);
} catch (e) {
install.recover(context)
return
install.recover(context);
return;
}
if (getConfig<boolean>('checkUpdates'))
install.checkUpdates(clangdPath, false, context);
Expand Down
18 changes: 10 additions & 8 deletions src/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ export async function checkUpdates(clangdPath: string, requested: boolean,
console.log('Failed to check for clangd update: ', e);
// We're not sure whether there's an upgrade: stay quiet unless asked.
if (requested)
vscode.window.showInformationMessage(
'Failed to check for clangd update: ' + e);
vscode.window.showErrorMessage('Failed to check for clangd update: ' + e);
return;
}
console.log('Checking for clangd update: available=', upgrade.new,
Expand Down Expand Up @@ -136,7 +135,7 @@ export async function latestRelease(): Promise<Release> {
const response = await fetch(url);
if (!response.ok) {
console.log(response.url, response.status, response.statusText);
throw new Error('Can\'t fetch release: ' + response.statusText);
throw new Error(`Can't fetch release: ${response.statusText}`);
}
return await response.json() as Release;
}
Expand All @@ -151,9 +150,9 @@ export function chooseAsset(release: Github.Release): Github.Asset|null {
const variant = variants[os.platform()];
// 32-bit vscode is still common on 64-bit windows, so don't reject that.
if (variant && (os.arch() == 'x64' || variant == 'windows'))
return release.assets.find(a => a.name.indexOf(variant) >= 0)
throw new Error(
`No clangd ${release.name} binary available for your platform`);
return release.assets.find(a => a.name.indexOf(variant) >= 0);
throw new Error(
`No clangd ${release.name} binary available for your platform`);
}
}

Expand All @@ -162,9 +161,9 @@ namespace End {
// The configuration is not valid, the user should install clangd manually.
export async function showInstallationHelp(message: string) {
const url = 'https://clangd.llvm.org/installation.html';
message += `\nSee ${url} for help.`
message += `\nSee ${url} for help.`;
if (await vscode.window.showErrorMessage(message, 'Open website'))
vscode.env.openExternal(vscode.Uri.parse(url));
vscode.env.openExternal(vscode.Uri.parse(url));
}

// We tried to install clangd, but something went wrong.
Expand Down Expand Up @@ -296,6 +295,9 @@ async function download(url: string, dest: string,
// We parse both github release numbers and installed `clangd --version` output
// by treating them as SemVer ranges, and offer an upgrade if the version
// is unambiguously newer.
//
// These functions throw if versions can't be parsed (e.g. installed clangd
// is a vendor-modified version).
namespace Version {
export async function upgrade(release: Github.Release, clangdPath: string) {
const releasedVer = released(release);
Expand Down

0 comments on commit 47d650a

Please sign in to comment.