Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure we use correct workspace folder for remote terminals #180486

Open
Tracked by #22879 ...
karrtikr opened this issue Apr 21, 2023 · 5 comments
Open
Tracked by #22879 ...

Ensure we use correct workspace folder for remote terminals #180486

karrtikr opened this issue Apr 21, 2023 · 5 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug remote Remote system operations issues terminal-process Problems launching processes, managing ptys, exiting, process leaks, etc.
Milestone

Comments

@karrtikr
Copy link
Contributor

karrtikr commented Apr 21, 2023

@Tyriar Last active workspace is not working properly, so getting workspace through cwd instead. Based on #179323 (comment) I was expecting it to be the same as cwd.

Smoke tests are timing out when attempting to use cwd workspace folder in remote terminals, not sure what might be going on. Here's our attempt: 9e4fa31

We can either do this or fix last active workspace so it points to correct workspace folder in multiroot scenarios.

@Tyriar
Copy link
Member

Tyriar commented May 25, 2023

Last active workspace is not working properly

Can you elaborate on what's not working exactly? Were the smoke tests failing because last active workspace was undefined? I think it's possible to be in a multi-root window but last active workspace being undefined.

@Tyriar Tyriar added the under-discussion Issue is under discussion for relevance, priority, approach label May 25, 2023
@Tyriar Tyriar added this to the May 2023 milestone May 25, 2023
@karrtikr
Copy link
Contributor Author

karrtikr commented May 25, 2023

There're two problems:

  • Last active workspace was not always pointing to the correct workspace folder, for eg. sometimes it would point to workspace1 when it should be workspace2. I was expecting it to be the same as cwd.
  • Hence I started using cwd to calculate the active workspace folder instead. But smoke tests fail for some reason when doing that, not sure about the reason.

@Tyriar
Copy link
Member

Tyriar commented May 25, 2023

Last active workspace was not always pointing to the correct workspace folder, for eg. sometimes it would point to workspace1 when it should be workspace2. I was expecting it to be the same as cwd.

Any repro on this? Here's what the history service does:

// Multiple folders: find the last active one
for (const input of this.getHistory()) {
if (isEditorInput(input)) {
continue;
}
if (schemeFilter && input.resource.scheme !== schemeFilter) {
continue;
}
const resourceWorkspace = this.contextService.getWorkspaceFolder(input.resource);
if (resourceWorkspace) {
return resourceWorkspace.uri;
}
}
// Fallback to first workspace matching scheme filter if any
for (const folder of folders) {
const resource = folder.uri;
if (!schemeFilter || resource.scheme === schemeFilter) {
return resource;
}
}
return undefined;

@karrtikr
Copy link
Contributor Author

  • Open a new terminal using the + icon on top right and select the 2nd workspace folder.
  • Put a break point inside getWorkspaceForTerminal function and compare values of cwd and lastActiveWorkspace

I'm not clear on what this.getHistory() is supposed to carry, does it include the 2nd workspace folder when it was selected through +?

@Tyriar Tyriar modified the milestones: May 2023, Backlog May 31, 2023
@karrtikr
Copy link
Contributor Author

@Tyriar FYI I can reproduce this in multi root workspace in codespaces, where scoped collections do not seem to work. That can be used to verify the fix.

@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug remote Remote system operations issues terminal-process Problems launching processes, managing ptys, exiting, process leaks, etc. and removed terminal Integrated terminal issues under-discussion Issue is under discussion for relevance, priority, approach labels Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug remote Remote system operations issues terminal-process Problems launching processes, managing ptys, exiting, process leaks, etc.
Projects
None yet
Development

No branches or pull requests

2 participants