Skip to content

Commit

Permalink
Merge pull request #190 from Techatrix/dev
Browse files Browse the repository at this point in the history
properly differentiate between lookup in PATH and an unset path option
  • Loading branch information
Vexu committed Apr 9, 2024
2 parents 4392e2b + d3538fb commit 35f63b5
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 32 deletions.
2 changes: 1 addition & 1 deletion package-lock.json

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

18 changes: 12 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,12 @@
},
"zig.path": {
"scope": "machine-overridable",
"type": "string",
"default": "",
"description": "Set a custom path to the Zig binary. Empty string will lookup zig in PATH."
"type": [
"string",
"null"
],
"default": null,
"description": "Set a custom path to the Zig binary. The string \"zig\" means lookup zig in PATH."
},
"zig.checkForUpdate": {
"scope": "resource",
Expand Down Expand Up @@ -164,9 +167,12 @@
},
"zig.zls.path": {
"scope": "machine-overridable",
"type": "string",
"default": "",
"description": "Path to `zls` executable. Example: `C:/zls/zig-cache/bin/zls.exe`. Empty string will lookup ZLS in PATH.",
"type": [
"string",
"null"
],
"default": null,
"description": "Path to `zls` executable. Example: `C:/zls/zig-cache/bin/zls.exe`. The string \"zls\" means lookup ZLS in PATH.",
"format": "path"
},
"zig.zls.enableSnippets": {
Expand Down
42 changes: 29 additions & 13 deletions src/zigSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,9 @@ async function checkUpdate(context: vscode.ExtensionContext) {

async function getUpdatedVersion(context: vscode.ExtensionContext): Promise<ZigVersion | null> {
const configuration = vscode.workspace.getConfiguration("zig");
const zigPath = configuration.get<string>("path");
if (zigPath) {
const zigBinPath = vscode.Uri.joinPath(context.globalStorageUri, "zig_install", "zig").fsPath;
if (!zigPath.startsWith(zigBinPath)) return null;
} else {
return null;
}
const zigPath = configuration.get<string | null>("path", null);
const zigBinPath = vscode.Uri.joinPath(context.globalStorageUri, "zig_install", "zig").fsPath;
if (!zigPath?.startsWith(zigBinPath)) return null;

const curVersion = getVersion(zigPath, "version");
if (!curVersion) return null;
Expand Down Expand Up @@ -240,6 +236,24 @@ export async function setupZig(context: vscode.ExtensionContext) {
await checkUpdate(context);
});

{
// convert an empty string for `zig.path` and `zig.zls.path` to `zig` and `zls` respectively.
// This check can be removed once enough time has passed so that most users switched to the new value

const zigConfig = vscode.workspace.getConfiguration("zig");
const initialSetupDone = zigConfig.get<boolean>("initialSetupDone", false);
const zigPath = zigConfig.get<string | null>("path");
if (zigPath === "" || (initialSetupDone && zigPath === null)) {
await zigConfig.update("path", "zig", true);
}

const zlsConfig = vscode.workspace.getConfiguration("zig.zls");
const zlsPath = zlsConfig.get<string | null>("path");
if (zlsPath === "" || (initialSetupDone && zlsPath === null)) {
await zlsConfig.update("path", "zls", true);
}
}

context.environmentVariableCollection.description = "Add Zig to PATH";
updateZigEnvironmentVariableCollection(context);
vscode.workspace.onDidChangeConfiguration((change) => {
Expand All @@ -253,7 +267,6 @@ export async function setupZig(context: vscode.ExtensionContext) {
await configuration.update("initialSetupDone", await initialSetup(context), true);
}

if (!configuration.get<string>("path")) return;
if (!configuration.get<boolean>("checkForUpdate")) return;
if (!(await shouldCheckUpdate(context, "zigUpdate"))) return;
await checkUpdate(context);
Expand All @@ -262,7 +275,7 @@ export async function setupZig(context: vscode.ExtensionContext) {
async function initialSetup(context: vscode.ExtensionContext): Promise<boolean> {
const zigConfig = vscode.workspace.getConfiguration("zig");

if (zigConfig.get<string>("path") === "") {
if (zigConfig.get<string | null>("path", null) === null) {
const zigResponse = await vscode.window.showInformationMessage(
"Zig path hasn't been set, do you want to specify the path or install Zig?",
{ modal: true },
Expand All @@ -273,7 +286,7 @@ async function initialSetup(context: vscode.ExtensionContext): Promise<boolean>
switch (zigResponse) {
case "Install":
await selectVersionAndInstall(context);
const zigPath = zigConfig.get<string>("path");
const zigPath = vscode.workspace.getConfiguration("zig").get<string | null>("path", null);
if (!zigPath) return false;
break;
case "Specify path":
Expand All @@ -291,16 +304,17 @@ async function initialSetup(context: vscode.ExtensionContext): Promise<boolean>
await zigConfig.update("path", uris[0].path, true);
break;
case "Use Zig in PATH":
await zigConfig.update("path", "", true);
await zigConfig.update("path", "zig", true);
break;
case undefined:
await zigConfig.update("path", undefined, true);
return false;
}
}

const zlsConfig = vscode.workspace.getConfiguration("zig.zls");

if (zlsConfig.get<string>("path") === "") {
if (zlsConfig.get<string | null>("path", null) === null) {
const zlsResponse = await vscode.window.showInformationMessage(
"We recommend enabling ZLS (the Zig Language Server) for a better editing experience. Would you like to install it?",
{ modal: true },
Expand All @@ -324,9 +338,11 @@ async function initialSetup(context: vscode.ExtensionContext): Promise<boolean>

await zlsConfig.update("path", uris[0].path, true);
case "Use ZLS in PATH":
await zlsConfig.update("path", "", true);
await zlsConfig.update("path", "zls", true);
break;
case undefined:
// explicitly set `zig.zls.path` to null so it is visible in the `settings.json`
await zlsConfig.update("path", null, true);
break;
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/zigUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export function getExePath(exePath: string | null, exeName: string, optionName:
}
}

if (!exePath) {
if (exePath === null) {
exePath = which.sync(exeName, { nothrow: true });
} else if (exePath.startsWith("~")) {
exePath = path.join(os.homedir(), exePath.substring(1));
Expand All @@ -30,7 +30,7 @@ export function getExePath(exePath: string | null, exeName: string, optionName:
}

let message;
if (!exePath) {
if (exePath === null) {
message = `Could not find ${exeName} in PATH`;
} else if (!fs.existsSync(exePath)) {
message = `\`${optionName}\` ${exePath} does not exist`;
Expand All @@ -48,8 +48,9 @@ export function getExePath(exePath: string | null, exeName: string, optionName:

export function getZigPath(): string {
const configuration = vscode.workspace.getConfiguration("zig");
const zigPath = configuration.get<string>("path") ?? null;
return getExePath(zigPath, "zig", "zig.path");
const zigPath = configuration.get<string | null>("path", null);
const exePath = zigPath !== "zig" ? zigPath : null; // the string "zig" means lookup in PATH
return getExePath(exePath, "zig", "zig.path");
}

// Check timestamp `key` to avoid automatically checking for updates
Expand Down
18 changes: 10 additions & 8 deletions src/zls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ export async function stopClient() {
// returns the file system path to the zls executable
export function getZLSPath(): string {
const configuration = vscode.workspace.getConfiguration("zig.zls");
const zlsPath = configuration.get<string>("path") ?? null;
return getExePath(zlsPath, "zls", "zig.zls.path");
const zlsPath = configuration.get<string | null>("path", null);
const exePath = zlsPath !== "zls" ? zlsPath : null; // the string "zls" means lookup in PATH
return getExePath(exePath, "zls", "zig.zls.path");
}

async function configurationMiddleware(
Expand Down Expand Up @@ -182,7 +183,7 @@ async function getVersionIndex(): Promise<VersionIndex> {
// checks whether there is newer version on master
async function checkUpdate(context: vscode.ExtensionContext) {
const configuration = vscode.workspace.getConfiguration("zig.zls");
const zlsPath = configuration.get<string>("path");
const zlsPath = configuration.get<string | null>("path", null);
const zlsBinPath = vscode.Uri.joinPath(context.globalStorageUri, "zls_install", "zls").fsPath;
if (!zlsPath) return;
if (!zlsPath.startsWith(zlsBinPath)) return;
Expand Down Expand Up @@ -325,13 +326,14 @@ async function installVersion(context: vscode.ExtensionContext, version: semver.
}

function checkInstalled(): boolean {
const zlsPath = vscode.workspace.getConfiguration("zig.zls").get<string>("path");
if (!zlsPath) {
const zlsPath = vscode.workspace.getConfiguration("zig.zls").get<string | null>("path", null);
if (zlsPath === null) {
void vscode.window.showErrorMessage("This command cannot be run without setting 'zig.zls.path'.", {
modal: true,
});
return false;
}
return !!zlsPath;
return true;
}

export async function activate(context: vscode.ExtensionContext) {
Expand Down Expand Up @@ -378,14 +380,14 @@ export async function activate(context: vscode.ExtensionContext) {
) {
await stopClient();
const zlsConfig = vscode.workspace.getConfiguration("zig.zls");
if (!!zlsConfig.get<string>("path")) {
if (zlsConfig.get<string | null>("path", null) !== null) {
await startClient();
}
}
}, context.subscriptions);

const zlsConfig = vscode.workspace.getConfiguration("zig.zls");
if (zlsConfig.get<string>("path") === undefined) return;
if (zlsConfig.get<string | null>("path", null) === null) return;
if (zlsConfig.get<boolean>("checkForUpdate") && (await shouldCheckUpdate(context, "zlsUpdate"))) {
await checkUpdate(context);
}
Expand Down

0 comments on commit 35f63b5

Please sign in to comment.