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

Ignore query string in detected terminal local links #181756

Merged
merged 4 commits into from May 8, 2023
Merged

Ignore query string in detected terminal local links #181756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bfbb2a5
Remove query string from local file links
Tyriar May 7, 2023
2f8e9cf
Add removeLinkQueryString tests
Tyriar May 7, 2023
9b15cca
Add some test edge cases
Tyriar May 7, 2023
e6210ec
Skip ? in unc paths
Tyriar May 8, 2023
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
16 changes: 15 additions & 1 deletion src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkParsing.ts
View file
Open in desktop
Expand Up @@ -105,7 +105,7 @@ function generateLinkSuffixRegex(eolOnly: boolean) {

/**
* Removes the optional link suffix which contains line and column information.
* @param link The link to parse.
* @param link The link to use.
*/
export function removeLinkSuffix(link: string): string {
const suffix = getLinkSuffix(link)?.suffix;
Expand All @@ -115,6 +115,20 @@ export function removeLinkSuffix(link: string): string {
return link.substring(0, suffix.index);
}

/**
* Removes any query string from the link.
* @param link The link to use.
*/
export function removeLinkQueryString(link: string): string {
// Skip ? in UNC paths
const start = link.startsWith('\\\\?\\') ? 4 : 0;
const index = link.indexOf('?', start);
if (index === -1) {
return link;
}
return link.substring(0, index);
}

/**
* Returns the optional link suffix which contains line and column information.
* @param link The link to parse.
Expand Down
11 changes: 8 additions & 3 deletions src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkResolver.ts
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { ITerminalLinkResolver, ResolvedLink } from 'vs/workbench/contrib/terminalContrib/links/browser/links';
import { removeLinkSuffix, winDrivePrefix } from 'vs/workbench/contrib/terminalContrib/links/browser/terminalLinkParsing';
import { removeLinkSuffix, removeLinkQueryString, winDrivePrefix } from 'vs/workbench/contrib/terminalContrib/links/browser/terminalLinkParsing';
import { URI } from 'vs/base/common/uri';
import { ITerminalBackend, ITerminalProcessManager } from 'vs/workbench/contrib/terminal/common/terminal';
import { Schemas } from 'vs/base/common/network';
Expand Down Expand Up @@ -52,9 +52,14 @@ export class TerminalLinkResolver implements ITerminalLinkResolver {
}
}

// Remove any line/col suffix before processing the path
// Remove any line/col suffix
let linkUrl = removeLinkSuffix(link);
if (!linkUrl) {

// Remove any query string
linkUrl = removeLinkQueryString(linkUrl);

// Exit early if the link is determines as not valid already
if (linkUrl.length === 0) {
cache.set(link, null);
return null;
}
Expand Down
15 changes: 14 additions & 1 deletion src/vs/workbench/contrib/terminalContrib/links/test/browser/terminalLinkParsing.test.ts
View file
Open in desktop
Expand Up @@ -5,7 +5,7 @@

import { deepStrictEqual, ok, strictEqual } from 'assert';
import { OperatingSystem } from 'vs/base/common/platform';
import { detectLinks, detectLinkSuffixes, getLinkSuffix, IParsedLink, removeLinkSuffix } from 'vs/workbench/contrib/terminalContrib/links/browser/terminalLinkParsing';
import { detectLinks, detectLinkSuffixes, getLinkSuffix, IParsedLink, removeLinkQueryString, removeLinkSuffix } from 'vs/workbench/contrib/terminalContrib/links/browser/terminalLinkParsing';

interface ITestLink {
link: string;
Expand Down Expand Up @@ -206,6 +206,19 @@ suite('TerminalLinkParsing', () => {
);
});
});
suite('removeLinkQueryString', () => {
test('should remove any query string from the link', () => {
strictEqual(removeLinkQueryString('?a=b'), '');
strictEqual(removeLinkQueryString('foo?a=b'), 'foo');
strictEqual(removeLinkQueryString('./foo?a=b'), './foo');
strictEqual(removeLinkQueryString('/foo/bar?a=b'), '/foo/bar');
strictEqual(removeLinkQueryString('foo?a=b?'), 'foo');
strictEqual(removeLinkQueryString('foo?a=b&c=d'), 'foo');
});
test('should respect ? in UNC paths', () => {
strictEqual(removeLinkQueryString('\\\\?\\foo?a=b'), '\\\\?\\foo',);
});
});
suite('detectLinks', () => {
test('foo(1, 2) bar[3, 4] "baz" on line 5', () => {
deepStrictEqual(
Expand Down