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

Ensure links resolve OS at detect/open time #176399

Merged
merged 1 commit into from Mar 7, 2023
Merged

Ensure links resolve OS at detect/open time #176399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkManager.ts
View file
Open in desktop
Expand Up @@ -83,7 +83,7 @@ export class TerminalLinkManager extends DisposableStore {
this._openers.set(TerminalBuiltinLinkType.LocalFile, localFileOpener);
this._openers.set(TerminalBuiltinLinkType.LocalFolderInWorkspace, localFolderInWorkspaceOpener);
this._openers.set(TerminalBuiltinLinkType.LocalFolderOutsideWorkspace, this._instantiationService.createInstance(TerminalLocalFolderOutsideWorkspaceLinkOpener));
this._openers.set(TerminalBuiltinLinkType.Search, this._instantiationService.createInstance(TerminalSearchLinkOpener, capabilities, this._processManager.initialCwd, localFileOpener, localFolderInWorkspaceOpener, this._processManager.os || OS));
this._openers.set(TerminalBuiltinLinkType.Search, this._instantiationService.createInstance(TerminalSearchLinkOpener, capabilities, this._processManager.initialCwd, localFileOpener, localFolderInWorkspaceOpener, () => this._processManager.os || OS));
this._openers.set(TerminalBuiltinLinkType.Url, this._instantiationService.createInstance(TerminalUrlLinkOpener, !!this._processManager.remoteAuthority));

this._registerStandardLinkProviders();
Expand Down
9 changes: 5 additions & 4 deletions src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkOpeners.ts
View file
Open in desktop
Expand Up @@ -78,7 +78,7 @@ export class TerminalSearchLinkOpener implements ITerminalLinkOpener {
private readonly _initialCwd: string,
private readonly _localFileOpener: TerminalLocalFileLinkOpener,
private readonly _localFolderInWorkspaceOpener: TerminalLocalFolderInWorkspaceLinkOpener,
private readonly _os: OperatingSystem,
private readonly _getOS: () => OperatingSystem,
@IFileService private readonly _fileService: IFileService,
@IInstantiationService private readonly _instantiationService: IInstantiationService,
@IQuickInputService private readonly _quickInputService: IQuickInputService,
Expand All @@ -89,7 +89,7 @@ export class TerminalSearchLinkOpener implements ITerminalLinkOpener {
}

async open(link: ITerminalSimpleLink): Promise<void> {
const osPath = osPathModule(this._os);
const osPath = osPathModule(this._getOS());
const pathSeparator = osPath.sep;
// Remove file:/// and any leading ./ or ../ since quick access doesn't understand that format
let text = link.text.replace(/^file:\/\/\/?/, '');
Expand Down Expand Up @@ -135,7 +135,8 @@ export class TerminalSearchLinkOpener implements ITerminalLinkOpener {

private async _getExactMatch(sanitizedLink: string): Promise<IResourceMatch | undefined> {
// Make the link relative to the cwd if it isn't absolute
const pathModule = osPathModule(this._os);
const os = this._getOS();
const pathModule = osPathModule(os);
const isAbsolute = pathModule.isAbsolute(sanitizedLink);
let absolutePath: string | undefined = isAbsolute ? sanitizedLink : undefined;
if (!isAbsolute && this._initialCwd.length > 0) {
Expand All @@ -146,7 +147,7 @@ export class TerminalSearchLinkOpener implements ITerminalLinkOpener {
let resourceMatch: IResourceMatch | undefined;
if (absolutePath) {
let normalizedAbsolutePath: string = absolutePath;
if (this._os === OperatingSystem.Windows) {
if (os === OperatingSystem.Windows) {
normalizedAbsolutePath = absolutePath.replace(/\\/g, '/');
if (normalizedAbsolutePath.match(/[a-z]:/i)) {
normalizedAbsolutePath = `/${normalizedAbsolutePath}`;
Expand Down
14 changes: 6 additions & 8 deletions src/vs/workbench/contrib/terminalContrib/links/browser/terminalLocalLinkDetector.ts
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { OperatingSystem, OS } from 'vs/base/common/platform';
import { OS } from 'vs/base/common/platform';
import { URI } from 'vs/base/common/uri';
import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity';
import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
Expand Down Expand Up @@ -51,8 +51,6 @@ export class TerminalLocalLinkDetector implements ITerminalLinkDetector {
// - Linux max length: 4096 ($PATH_MAX)
readonly maxLinkLength = 500;

private _os: OperatingSystem;

constructor(
readonly xterm: Terminal,
private readonly _capabilities: ITerminalCapabilityStore,
Expand All @@ -61,7 +59,6 @@ export class TerminalLocalLinkDetector implements ITerminalLinkDetector {
@IUriIdentityService private readonly _uriIdentityService: IUriIdentityService,
@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService
) {
this._os = _processManager.os || OS;
}

async detect(lines: IBufferLine[], startLine: number, endLine: number): Promise<ITerminalSimpleLink[]> {
Expand All @@ -76,7 +73,8 @@ export class TerminalLocalLinkDetector implements ITerminalLinkDetector {
let stringIndex = -1;
let resolvedLinkCount = 0;

const parsedLinks = detectLinks(text, this._os);
const os = this._processManager.os || OS;
const parsedLinks = detectLinks(text, os);
for (const parsedLink of parsedLinks) {
// Don't try resolve any links of excessive length
if (parsedLink.path.text.length > Constants.MaxResolvedLinkLength) {
Expand All @@ -93,12 +91,12 @@ export class TerminalLocalLinkDetector implements ITerminalLinkDetector {

// Get a single link candidate if the cwd of the line is known
const linkCandidates: string[] = [];
if (osPathModule(this._os).isAbsolute(parsedLink.path.text) || parsedLink.path.text.startsWith('~')) {
const osPath = osPathModule(os);
if (osPath.isAbsolute(parsedLink.path.text) || parsedLink.path.text.startsWith('~')) {
linkCandidates.push(parsedLink.path.text);
} else {
if (this._capabilities.has(TerminalCapability.CommandDetection)) {
const osModule = osPathModule(this._os);
const absolutePath = updateLinkWithRelativeCwd(this._capabilities, bufferRange.start.y, parsedLink.path.text, osModule);
const absolutePath = updateLinkWithRelativeCwd(this._capabilities, bufferRange.start.y, parsedLink.path.text, osPath);
// Only add a single exact link candidate if the cwd is available, this may cause
// the link to not be resolved but that should only occur when the actual file does
// not exist. Doing otherwise could cause unexpected results where handling via the
Expand Down
20 changes: 10 additions & 10 deletions src/vs/workbench/contrib/terminalContrib/links/test/browser/terminalLinkOpeners.test.ts
View file
Open in desktop
Expand Up @@ -125,7 +125,7 @@ suite('Workbench - TerminalLinkOpeners', () => {
test('should open single exact match against cwd when searching if it exists when command detection cwd is available', async () => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, OperatingSystem.Linux);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, () => OperatingSystem.Linux);
// Set a fake detected command starting as line 0 to establish the cwd
commandDetection.setCommands([{
command: '',
Expand Down Expand Up @@ -156,7 +156,7 @@ suite('Workbench - TerminalLinkOpeners', () => {
test('should open single exact match against cwd for paths containing a separator when searching if it exists, even when command detection isn\'t available', async () => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, OperatingSystem.Linux);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, () => OperatingSystem.Linux);
fileService.setFiles([
URI.from({ scheme: Schemas.file, path: '/initial/cwd/foo/bar.txt' }),
URI.from({ scheme: Schemas.file, path: '/initial/cwd/foo2/bar.txt' })
Expand All @@ -175,7 +175,7 @@ suite('Workbench - TerminalLinkOpeners', () => {
test('should open single exact match against any folder for paths not containing a separator when there is a single search result, even when command detection isn\'t available', async () => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, OperatingSystem.Linux);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, () => OperatingSystem.Linux);
capabilities.remove(TerminalCapability.CommandDetection);
opener.setFileQueryBuilder({ file: () => null! });
fileService.setFiles([
Expand All @@ -202,7 +202,7 @@ suite('Workbench - TerminalLinkOpeners', () => {
test('should open single exact match against any folder for paths not containing a separator when there are multiple search results, even when command detection isn\'t available', async () => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, OperatingSystem.Linux);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, () => OperatingSystem.Linux);
capabilities.remove(TerminalCapability.CommandDetection);
opener.setFileQueryBuilder({ file: () => null! });
fileService.setFiles([
Expand Down Expand Up @@ -232,7 +232,7 @@ suite('Workbench - TerminalLinkOpeners', () => {
test('should not open single exact match for paths not containing a when command detection isn\'t available', async () => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, OperatingSystem.Linux);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/initial/cwd', localFileOpener, localFolderOpener, () => OperatingSystem.Linux);
fileService.setFiles([
URI.from({ scheme: Schemas.file, path: '/initial/cwd/foo/bar.txt' }),
URI.from({ scheme: Schemas.file, path: '/initial/cwd/foo2/bar.txt' })
Expand All @@ -252,7 +252,7 @@ suite('Workbench - TerminalLinkOpeners', () => {
setup(() => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '', localFileOpener, localFolderOpener, OperatingSystem.Linux);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '', localFileOpener, localFolderOpener, () => OperatingSystem.Linux);
});

test('should apply the cwd to the link only when the file exists and cwdDetection is enabled', async () => {
Expand Down Expand Up @@ -309,7 +309,7 @@ suite('Workbench - TerminalLinkOpeners', () => {
test('should extract line and column from links in a workspace containing spaces', async () => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/space folder', localFileOpener, localFolderOpener, OperatingSystem.Linux);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '/space folder', localFileOpener, localFolderOpener, () => OperatingSystem.Linux);
fileService.setFiles([
URI.from({ scheme: Schemas.file, path: '/space folder/foo/bar.txt' })
]);
Expand All @@ -333,13 +333,13 @@ suite('Workbench - TerminalLinkOpeners', () => {
setup(() => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '', localFileOpener, localFolderOpener, OperatingSystem.Windows);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, '', localFileOpener, localFolderOpener, () => OperatingSystem.Windows);
});

test('should apply the cwd to the link only when the file exists and cwdDetection is enabled', async () => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, 'c:\\Users', localFileOpener, localFolderOpener, OperatingSystem.Windows);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, 'c:\\Users', localFileOpener, localFolderOpener, () => OperatingSystem.Windows);

const cwd = 'c:\\Users\\home\\folder';
const absoluteFile = 'c:\\Users\\home\\folder\\file.txt';
Expand Down Expand Up @@ -394,7 +394,7 @@ suite('Workbench - TerminalLinkOpeners', () => {
test('should extract line and column from links in a workspace containing spaces', async () => {
localFileOpener = instantiationService.createInstance(TerminalLocalFileLinkOpener);
const localFolderOpener = instantiationService.createInstance(TerminalLocalFolderInWorkspaceLinkOpener);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, 'c:/space folder', localFileOpener, localFolderOpener, OperatingSystem.Windows);
opener = instantiationService.createInstance(TestTerminalSearchLinkOpener, capabilities, 'c:/space folder', localFileOpener, localFolderOpener, () => OperatingSystem.Windows);
fileService.setFiles([
URI.from({ scheme: Schemas.file, path: 'c:/space folder/foo/bar.txt' })
]);
Expand Down