Skip to content
91 changes: 91 additions & 0 deletions src/LanguageServerManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,97 @@ describe('LanguageServerManager', () => {
);
});

it('resolves relative bsdk path from the workspace file directory, not the project root', async () => {
// .code-workspace file lives in a subdirectory (.vscode/)
vscode.workspace.workspaceFile = URI.file(s`${tempDir}/.vscode/workspace.code-workspace`);
vscode.workspace.workspaceFolders.push({
index: 0,
name: 'SDK',
uri: URI.file(s`${tempDir}`)
});

setConfig(vscode.workspace.workspaceFile.fsPath, {
'brightscript.bsdk': '../node_modules/brighterscript'
});

expect(
s(await languageServerManager['getBsdkVersionInfo']())
).to.eql(
// resolves from .vscode/ → one level up → tempDir/node_modules/brighterscript
s`${tempDir}/node_modules/brighterscript`
);
});

it('expands ${workspaceFolder:name} variable in bsdk path', async () => {
vscode.workspace.workspaceFile = URI.file(s`${tempDir}/.vscode/workspace.code-workspace`);
vscode.workspace.workspaceFolders.push({
index: 0,
name: 'SDK',
uri: URI.file(s`${tempDir}`)
});

setConfig(vscode.workspace.workspaceFile.fsPath, {
'brightscript.bsdk': '${workspaceFolder:SDK}/node_modules/brighterscript'
});

expect(
s(await languageServerManager['getBsdkVersionInfo']())
).to.eql(s`${tempDir}/node_modules/brighterscript`);
});

it('expands ${workspaceFolder} variable to the first workspace folder', async () => {
vscode.workspace.workspaceFile = URI.file(s`${tempDir}/.vscode/workspace.code-workspace`);
vscode.workspace.workspaceFolders.push({
index: 0,
name: 'SDK',
uri: URI.file(s`${tempDir}`)
});

setConfig(vscode.workspace.workspaceFile.fsPath, {
'brightscript.bsdk': '${workspaceFolder}/node_modules/brighterscript'
});

expect(
s(await languageServerManager['getBsdkVersionInfo']())
).to.eql(s`${tempDir}/node_modules/brighterscript`);
});

it('throws for an unrecognized variable in bsdk path', async () => {
vscode.workspace.workspaceFile = URI.file(s`${tempDir}/.vscode/workspace.code-workspace`);
vscode.workspace.workspaceFolders.push({
index: 0,
name: 'SDK',
uri: URI.file(s`${tempDir}`)
});

setConfig(vscode.workspace.workspaceFile.fsPath, {
'brightscript.bsdk': '${env:MY_VAR}/node_modules/brighterscript'
});

await expectThrowsAsync(
() => languageServerManager['getBsdkVersionInfo'](),
'brightscript.bsdk: unsupported variable in bsdk "${env:MY_VAR}/node_modules/brighterscript"'
);
});

it('throws for an unknown workspace folder name in bsdk path', async () => {
vscode.workspace.workspaceFile = URI.file(s`${tempDir}/.vscode/workspace.code-workspace`);
vscode.workspace.workspaceFolders.push({
index: 0,
name: 'SDK',
uri: URI.file(s`${tempDir}`)
});

setConfig(vscode.workspace.workspaceFile.fsPath, {
'brightscript.bsdk': '${workspaceFolder:Unknown}/node_modules/brighterscript'
});

await expectThrowsAsync(
() => languageServerManager['getBsdkVersionInfo'](),
'brightscript.bsdk: unknown workspace folder name "Unknown"'
);
});

it('returns folder version when not in a workspace', async () => {
vscode.workspace.workspaceFolders.push({
index: 0,
Expand Down
55 changes: 53 additions & 2 deletions src/LanguageServerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -524,17 +524,18 @@ export class LanguageServerManager {
//use bsdk entry in the code-workspace file
if (this.workspaceConfigIncludesBsdkKey()) {
let result = this.parseVersionInfo(
util.getConfiguration('brightscript', vscode.workspace.workspaceFile).get<string>('bsdk')?.trim?.(),
this.getWorkspaceBsdkInfo(vscode.workspace.workspaceFile),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a fan of mixing workspaceFile and per-folder logic into one function. Should be:

  • collect each folder's settings value
  • collect the code-workspace value
  • if code-workspace value exists, use it; else fall back to folder values

Right now that's inter-mixed inside getWorkspaceBsdkInfo's folder loop, so it's blurry (and weird, because in this case I think every folder's value would be identical since we're defaulting to the code-workspace's value). I'd expect the .code-workspace handling to live as separate logic after the per-folder loop, not merged into it.

Something more like this:

Image

path.dirname(vscode.workspace.workspaceFile.fsPath)
);

if (result) {
return result.value;
}
}

//collect `brightscript.bsdk` setting value from each workspaceFolder
const folderResults = vscode.workspace.workspaceFolders?.reduce((acc, workspaceFolder) => {
const versionInfo = util.getConfiguration('brightscript', workspaceFolder).get<string>('bsdk');
const versionInfo = this.getWorkspaceBsdkInfo(workspaceFolder);
const parsed = this.parseVersionInfo(versionInfo, workspaceFolder.uri.fsPath);
if (parsed) {
acc.set(parsed.value, parsed);
Expand All @@ -557,6 +558,56 @@ export class LanguageServerManager {
}
}

/**
* Get the `brightscript.bsdk` value from the .code-workspace settings block with workspace
* folder variables expanded. `inspect().workspaceValue` returns the unresolved string from
* the .code-workspace file — VSCode does not expand variables like ${workspaceFolder} in this
* value, so we expand ${workspaceFolder} and ${workspaceFolder:name} ourselves.
*/
private getWorkspaceBsdkInfo(workspaceFolder: vscode.ConfigurationScope) {
const config = util.getConfiguration('brightscript', workspaceFolder);
const rawValue = config.inspect<string>('bsdk')?.workspaceValue?.trim?.() ?? config.get<string>('bsdk')?.trim?.();

const hasVariable = rawValue?.startsWith('${');
if (!hasVariable) {
return rawValue;
}

return this.expandWorkspaceBsdkInfo(rawValue);
}

/**
* Expand ${workspaceFolder} and ${workspaceFolder:name} variables in a bsdk path string.
* VSCode does not expand these variables in inspect().workspaceValue, so we do it ourselves.
* Throws if an unrecognized variable is encountered.
*/
private expandWorkspaceBsdkInfo(value: string): string {
if (!value) {
return value;
}

const [match, workspaceName, relativePath] = /^\$\{workspaceFolder:?([^}]*)\}(.*)$/.exec(value) ?? [];

if (!match) {
throw new Error(`brightscript.bsdk: unsupported variable in bsdk "${value}"`);
}

// if ${workspaceFolder}, use workspaceFolders[0]
let workspaceFolder = vscode.workspace.workspaceFolders?.[0];

// if ${workspaceFolder:name}, find by name
if (workspaceName) {
workspaceFolder = vscode.workspace.workspaceFolders?.find(f => f.name === workspaceName);
}

if (!workspaceFolder) {
throw new Error(`brightscript.bsdk: unknown workspace folder name "${workspaceName}"`);
}

const bsdkInfo = path.join(workspaceFolder.uri.fsPath, relativePath);
return bsdkInfo ?? '';
}

private workspaceConfigIncludesBsdkKey() {
return vscode.workspace.workspaceFile &&
fsExtra.pathExistsSync(vscode.workspace.workspaceFile.fsPath) &&
Expand Down
3 changes: 2 additions & 1 deletion src/mockVscode.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ export let vscode = {
inspect: (name: string) => {
return {
key: name,
globalValue: store?.[`${configurationName}.${name}`]
globalValue: store?.[`${configurationName}.${name}`],
workspaceValue: store?.[`${configurationName}.${name}`]
} as ReturnType<WorkspaceConfiguration['inspect']>;
},
update: (name: string, value: any) => {
Expand Down
Loading