Skip to content

[fix] resolve brightscript.bsdk correctly when .code-workspace is not at project root - #878

Open
addison-adler wants to merge 9 commits into
rokucommunity:masterfrom
addison-adler:fix/bsdk-workspace-file-path-resolution
Open

[fix] resolve brightscript.bsdk correctly when .code-workspace is not at project root#878
addison-adler wants to merge 9 commits into
rokucommunity:masterfrom
addison-adler:fix/bsdk-workspace-file-path-resolution

Conversation

@addison-adler

Copy link
Copy Markdown
Contributor

Summary

Context

  • getConfiguration(...).get('bsdk') pre-resolves relative paths and variables like ${workspaceFolder} against the wrong base when the .code-workspace file is not at the project root
    • ${workspaceFolder}/node_modules/brighterscript gets resolved to node_modules/brighterscript
  • then, LanguageServerManager.ts uses the workspace file as the cwd location

net result:

  • .code-workspace doesn't work anywhere except for the project root

Changes

Updated behavior:

  • if brightscript.bsdk starts with ${workspaceFolder}, resolve relative to the first workspace folder (same behavior as VSCode)
  • if brightscript.bsdk starts with ${workspaceFolder:name}, resolve relative to the named workspace folder
  • else, same behavior as previously!

How I Tested

  • built the extension locally,
  • tested ${workspaceFolder}, ${workspaceFolder}, and plain string path
  • tested both relative and absolute paths

Test plan

  • Place .code-workspace file in a subdirectory (e.g. .vscode/) with a relative brightscript.bsdk path — language server should resolve correctly
  • Use ${workspaceFolder:FolderName}/node_modules/brighterscript — should resolve to the named folder's path
  • Use an unrecognized variable like ${env:MY_VAR} — should surface a clear error in the BrightScript output panel
  • Single-root workspace (no .code-workspace file) — behavior unchanged

addison-adler and others added 6 commits August 13, 2026 07:43
…at project root

VSCode's getConfiguration pre-resolves relative paths and variables like
${workspaceFolder} against the wrong base when the .code-workspace file is
not at the project root. Use inspect().workspaceValue to get the raw
unresolved string, then expand ${workspaceFolder} and ${workspaceFolder:name}
variables ourselves before passing to parseVersionInfo.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ce files

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread src/LanguageServerManager.ts Outdated
Comment thread src/LanguageServerManager.ts Outdated
addison-adler and others added 3 commits August 16, 2026 20:18
Co-authored-by: Addison <152139745+addison-adler@users.noreply.github.com>
…ce loop

The PR accidentally changed the per-folder config read to always fetch
from vscode.workspace.workspaceFile instead of each workspaceFolder.
This meant multi-folder workspaces never accumulated distinct bsdk
values, so selectBrighterScriptVersion was never called.
…, add get() fallback

In the folderResults reduce loop, getWorkspaceBsdkInfo was incorrectly
called with vscode.workspace.workspaceFile (always the same global
URI) instead of the current workspaceFolder, so all folders got the
same — usually undefined — value and selectBrighterScriptVersion was
never reached.

Also extend getWorkspaceBsdkInfo to fall back to get() when
inspect().workspaceValue is not set: workspaceValue carries the raw
unresolved string from the .code-workspace file (needed for
${workspaceFolder} expansion), while per-folder settings have no
workspaceValue and must be read via get().

@TwitchBronBron TwitchBronBron left a comment

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.

I like the ${workspaceFolder} and ${workspaceFolder:name} concept. But I think the current logic needs a little bit of refactoring to keep the logic a little easier to follow.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants