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.

Sign up for GitHub

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

Jump to bottom

Links to relative files don't work if the workspace has a file of the same name #141502

Closed
connor4312 opened this issue Jan 25, 2022 · 10 comments · Fixed by #141595
Closed

Links to relative files don't work if the workspace has a file of the same name #141502

connor4312 opened this issue Jan 25, 2022 · 10 comments · Fixed by #141595
Assignees
@Tyriar
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-links terminal-shell-integration Shell integration, command decorations, etc. verified Verification succeeded
Milestone
February 2022

Comments

@connor4312
Copy link
Member

connor4312 commented Jan 25, 2022 •
edited

Testing #141339

  1. Have vscode open to folder a, and another folder b. Have the files a/foo.txt, a/bar.txt, and b/foo.txt, b/baz.txt
  2. cd to folder b
  3. enter ./baz.txt in the terminal and click on it. b/baz.txt opens ✅
  4. enter foo.txt in the folder. a/foo.txt opens 🐛
22-01-73a34ec4-77d1-4847-ab70-0c4ab501263b.mp4
@meganrogge meganrogge added bug Issue identified by VS Code Team member as probable bug terminal-links labels Jan 26, 2022
@meganrogge meganrogge added this to the January 2022 milestone Jan 26, 2022
@meganrogge
Copy link
Contributor

Maybe I'm doing something wrong, but I can't reproduce. I have a workspace with a package.json.

It opens that before I cd into a new directory. I cd into a new directory with a package.json and when I click on it, that one opens.
Recording 2022-01-25 at 18 10 00

@meganrogge
Copy link
Contributor

did it look like shell integration was enabled on hover @connor4312 ?

@Tyriar
Copy link
Member

Tyriar commented Jan 26, 2022

Can repro, we need to also make validated local link provider aware of command detection.

@Tyriar Tyriar removed their assignment Jan 26, 2022
@meganrogge
Copy link
Contributor

this should already work as preprocessPath uses the cwd

vscode/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts

Lines 371 to 404 in f695611

if (!this._processManager) {
throw new Error('Process manager is required');
}
if (link.charAt(0) === '~') {
// Resolve ~ -> userHome
if (!this._processManager.userHome) {
return null;
}
link = this._osPath.join(this._processManager.userHome, link.substring(1));
} else if (link.charAt(0) !== '/' && link.charAt(0) !== '~') {
// Resolve workspace path . | .. | <relative_path> -> <path>/. | <path>/.. | <path>/<relative_path>
if (this._processManager.os === OperatingSystem.Windows) {
if (!link.match('^' + winDrivePrefix) && !link.startsWith('\\\\?\\')) {
if (!this._processCwd) {
// Abort if no workspace is open
return null;
}
link = this._osPath.join(this._processCwd, link);
} else {
// Remove \\?\ from paths so that they share the same underlying
// uri and don't open multiple tabs for the same file
link = link.replace(/^\\\\\?\\/, '');
}
} else {
if (!this._processCwd) {
// Abort if no workspace is open
return null;
}
link = this._osPath.join(this._processCwd, link);
}
}
link = this._osPath.normalize(link);
return link;

Guessing this is a windows only issue, as I'm not encountering it on mac

@Tyriar
Copy link
Member

Tyriar commented Jan 26, 2022

@meganrogge I think _processCwd is hooked up to the old system:

vscode/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

Line 1482 in f7b1711

this._updateProcessCwd();

meganrogge added a commit that referenced this issue Jan 26, 2022
@meganrogge
fix #141502
1f65fd1
@meganrogge meganrogge mentioned this issue Jan 26, 2022
Merged
@rebornix rebornix added the verified Verification succeeded label Jan 28, 2022
@rebornix
Copy link
Member

image

@meganrogge is this as expected, I'm in folder b and clicking on foo.txt opens the quick pick other than directly opening b/foo.txt.

@rebornix rebornix added verification-found Issue verification failed and removed verified Verification succeeded labels Jan 28, 2022
@meganrogge
Copy link
Contributor

meganrogge commented Jan 28, 2022 •
edited

@rebornix if you create another terminal and hover the tab, what capabilities do you see?

@meganrogge meganrogge removed the verification-found Issue verification failed label Jan 28, 2022
@meganrogge meganrogge reopened this Jan 28, 2022
@meganrogge
Copy link
Contributor

@rebornix hadn't enabled shell integration. he did and does see the capabilities, the run recent command works, but he said that this does not.

@meganrogge meganrogge reopened this Jan 28, 2022
@meganrogge
Copy link
Contributor

@rebornix's issue above is with word links whereas this original issue was not

@Tyriar
Copy link
Member

Tyriar commented Feb 7, 2022

Can repro, the following screenshot tries to open a link in vscode:

image
image

@Tyriar Tyriar added the terminal-shell-integration Shell integration, command decorations, etc. label Feb 9, 2022
@Tyriar Tyriar closed this as completed in 9f32b2c Feb 9, 2022
@connor4312 connor4312 added the verified Verification succeeded label Feb 23, 2022
@connor4312 connor4312 mentioned this issue Feb 23, 2022
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Tyriar Tyriar

Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-links terminal-shell-integration Shell integration, command decorations, etc. verified Verification succeeded
Projects
None yet
Milestone
February 2022
Development

Successfully merging a pull request may close this issue.

update terminalLinkManager's processCwd when cwd changes with shell integration on microsoft/vscode
5 participants
@rebornix @Tyriar @connor4312 @meganrogge and others