Skip to content

Conversation

@steveclarke
Copy link

Multi-Root Workspace Support

Summary

Each workspace folder now gets its own language server with its own Standard Ruby configuration. This fixes false positives in monorepos where different folders have different configs (e.g., a Rails app with standard-rails alongside a Ruby library with plain standard).

Closes #9

A Note on How This Was Developed

I used AI assistance to help write this refactor, but I want to be clear about my involvement: I personally read the original codebase and understood how it works. I reviewed every line of the refactored code, and I compiled detailed notes to ensure I understand what changed and why. This isn't a blind "get the AI to do something and ship it" situation - I have a reasonable understanding of how the extension works and how this refactor achieves multi-root workspace support. Feel free to ask me any questions about the implementation.

Architecture: New ClientManager Class

The server lifecycle logic was extracted into a new ClientManager class (src/clientManager.ts). This class:

  • Manages one language client per workspace folder
  • Tracks per-folder state: clients, diagnostic caches, file watchers
  • Handles workspace folder add/remove events
  • Properly cleans up resources (watchers, caches) when servers stop
Original ClientManager Method(s)
startLanguageServer ClientManager.startForFolder, ClientManager.startAll
stopLanguageServer ClientManager.stopForFolder, ClientManager.stopAll
restartLanguageServer ClientManager.restartAll
afterStartLanguageServer ClientManager.afterStart
syncOpenDocumentsWithLanguageServer ClientManager.syncOpenDocuments

The class is injected with callbacks (createClient, shouldEnableForFolder, etc.) so it doesn't contain Standard Ruby-specific logic - it's purely about managing multiple language clients.

Key Functional Changes

buildLanguageClientOptions()

This function has the most significant changes to enable per-folder scoping:

Aspect Original Refactored
documentSelector Global Folder-scoped with glob pattern
diagnosticCollectionName 'standardRuby' 'standardRuby-${folder.name}'
workspaceFolder Not set Set to folder
File watchers Global, never disposed Folder-scoped, registered for cleanup
Diagnostic cache Single global Map Per-folder cache from ClientManager

A new normalizePathForGlob() helper handles Windows path compatibility.

syncOpenDocuments()

The original synced ALL open Ruby documents to the single server. The refactored version checks workspace.getWorkspaceFolder(doc.uri) and only syncs documents that belong to the specific folder.

Bug Fixes

File System Watcher Cleanup

The original code created file system watchers inline but never disposed them - when the server stopped, watchers remained active (resource leak). The refactored code registers watchers with ClientManager.registerWatchers(), which tracks them per folder. When a folder's server stops, the watchers are properly disposed. This is especially important for multi-root workspaces where folders may be added/removed dynamically.

${cwd} Resolution Inconsistency

resolveCommandPath() originally resolved ${cwd} to process.cwd() (Node's process directory), but commands actually ran in the workspace folder's path. These could differ depending on how VS Code was launched. The refactored version resolves ${cwd} to folder.uri.fsPath, making it consistent with command execution.

Minor Changes

Most utility functions now take a folder: WorkspaceFolder parameter instead of using getCwd() (which always returned the first workspace folder). The getCwd() helper was removed.

Function Notes
displayBundlerError
isValidBundlerProject
isInBundle
shouldEnableForFolder Renamed from shouldEnableExtension
resolveCommandPath
getCommand
supportedVersionOfStandard
buildExecutable
buildLanguageClientOptions
createLanguageClient

VS Code Engine Version

Updated the minimum VS Code version in package.json from ^1.75.0 to ^1.102.0.

This fixes a pre-existing issue: the upstream repo had @types/vscode at ^1.102.0 but engines.vscode at ^1.75.0. This mismatch causes vsce package to fail:

ERROR  @types/vscode ^1.102.0 greater than engines.vscode ^1.75.0

Testing

  • All existing tests pass
  • New unit tests for ClientManager (src/test/suite/clientManager.test.ts) covering:
    • Client lifecycle (start, stop, restart)
    • Multiple folders with separate clients
    • Race condition prevention
    • Watcher cleanup
    • Error handling

Test Fixture

I'm not sure how to write an automated test for a VS Code extension against a multi-root workspace fixture. For now, I've included a manual test fixture that you should be able to use to verify the fix. If you have any advice on how to automate this, I'm open to giving it a try.

A test fixture is included at test-fixtures/multi-root-demo/ (see the README there for detailed instructions):

test-fixtures/multi-root-demo/
├── multi-root-demo.code-workspace
├── README.md
├── app-rails/                        # Uses standard-rails (first alphabetically)
│   └── .standard.yml
└── ruby-lib/                         # Uses plain standard
    └── lib/string_utils.rb

To test:

  1. Open multi-root-demo.code-workspace with the published extension - you should see Rails/Output errors on ruby-lib/lib/string_utils.rb (this reproduces the bug)
  2. Press F5 to launch Extension Development Host with this branch
  3. Open the same workspace - the errors should be gone (this verifies the fix)
  4. Check Output panel - should see TWO language servers starting (one per folder)

Question

I kept getLanguageClient() for backward compatibility with the original languageClient export, but I'm not sure if anything external uses it. Should I remove it? The tests could be updated to use languageClients directly instead.

Each workspace folder now gets its own language server with its own
Standard Ruby configuration. This fixes false positives in monorepos
where different folders have different configs (e.g., Rails app with
standard-rails alongside a Ruby library with plain standard).

Changes:
- Extract ClientManager class to manage per-folder language clients
- Add workspace folder change listener (start/stop servers on add/remove)
- Scope document selectors, diagnostics, and file watchers per folder
- Fix resource leak: file watchers now properly disposed on server stop
- Fix ${cwd} resolution inconsistency in command paths
- Align engines.vscode with @types/vscode (^1.102.0)

Closes standardrb#9
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.

Customizing CWD for nested workspaces

1 participant