Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Fallback to guessing the active workspace if none is found based on URI #976

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

vinistock
Copy link
Member

Motivation

Closes Shopify/ruby-lsp#1689

I believe I understand the problem some users are having. In #950, we changed the logic to not try to guess the active workspace anymore and just rely on the folder passed by VS Code itself.

However, I think this doesn't work for monorepos that place their launch.json configurations on the top level. This project structure:

monorepo/
  .vscode/launch.json
  client/
  server/

For this scenario, even with the multi-root workspace configuration, VS Code gives us the URI for the top level when launching the debugger because that's where the debugging task is configured. But since that's not a Ruby workspace, we never find anything.

If we fallback to guessing the active workspace, we should be able to account for this scenario better.

Implementation

Created a new method to resolve the workspace that accepts a nilable URI. The key difference in behaviour is that if the URI is present, but we can't find a workspace for it, we now fallback to trying to figure out the active workspace (which we weren't doing).

@vinistock vinistock added the bugfix This PR will fix an existing bug label Jan 11, 2024
@vinistock vinistock self-assigned this Jan 11, 2024
@vinistock vinistock requested a review from a team as a code owner January 11, 2024 14:32
src/rubyLsp.ts Show resolved Hide resolved
@vinistock
Copy link
Member Author

Apologies for adding more things after approval, but I noticed that we could still improve the spawning of the debugger to be less dependent on the workspace.

I tested the latest commit with a single and a multi workspace configuration and everything seemed to work properly.

src/debugger.ts Outdated
let workspaceName = session.workspaceFolder?.name;

if (!workspaceName) {
workspaceName = cwd ? path.basename(cwd) : "unknown";
Copy link
Member

Choose a reason for hiding this comment

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

Should we error out when the workspace name is unknown? Is it possible to enter this case but still can use the debugger correctly 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point. I started saving our resolved workspace in the debug session configuration instead, so that we can reuse it when spawning the debugger.

That way, the error handling happens when resolving debug configurations and spawning just uses the information saved directly.

@vinistock vinistock force-pushed the vs/fallback_to_active_workspace branch from bea53bc to 9f42a97 Compare January 12, 2024 14:13
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

👍

@vinistock vinistock merged commit 79d5355 into main Jan 12, 2024
10 checks passed
@vinistock vinistock deleted the vs/fallback_to_active_workspace branch January 12, 2024 16:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bugfix This PR will fix an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debugging requires a workspace folder to be opened
3 participants