Skip to content

Commit

Permalink
Fixes #2625 adds # escaping to regex
Browse files Browse the repository at this point in the history 
Ensures correct repository on GitHub/GitLab enriched autolinks
  • Loading branch information
@eamodio
eamodio committed Oct 27, 2023
1 parent 8aa81e9 commit 179b1bf
Show file tree
Hide file tree
Showing 6 changed files with 227 additions and 41 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p

### Fixed

- Fixes [#2625](https://github.com/gitkraken/vscode-gitlens/issues/2625) - full issue ref has escape characters that break hover links
- Fixes [#2987](https://github.com/gitkraken/vscode-gitlens/issues/2987) - Unable to remove all marks on reviewed files with a single operation
- Fixes [#2923](https://github.com/gitkraken/vscode-gitlens/issues/2923) - TypeError: Only absolute URLs are supported
- Fixes [#2926](https://github.com/gitkraken/vscode-gitlens/issues/2926) - "Open File at Revision" has incorrect editor label if revision contains path separator
Expand All @@ -44,6 +45,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
- Fixes intermittent issues where details sometimes get cleared/overwritten when opening the _Commit Details_ view
- Fixes issue when clicking on commits in the Visual File History to open the _Commit Details_ view
- Fixes issue opening stashes in the _Commit Details_ view from the _Stashes_ view
- Fixes issue where GitHub/GitLab enriched autolinks could incorrectly point to the wrong repository

## [14.4.0] - 2023-10-13

Expand Down
6 changes: 4 additions & 2 deletions src/annotations/autolinks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { IssueOrPullRequest } from '../git/models/issue';
import { getIssueOrPullRequestHtmlIcon, getIssueOrPullRequestMarkdownIcon } from '../git/models/issue';
import type { GitRemote } from '../git/models/remote';
import type { RemoteProviderReference } from '../git/models/remoteProvider';
import type { RichRemoteProvider } from '../git/remotes/richRemoteProvider';
import type { RepositoryDescriptor, RichRemoteProvider } from '../git/remotes/richRemoteProvider';
import type { MaybePausedResult } from '../system/cancellation';
import { configuration } from '../system/configuration';
import { fromNow } from '../system/date';
Expand All @@ -30,6 +30,8 @@ export interface Autolink {

type?: AutolinkType;
description?: string;

descriptor?: RepositoryDescriptor;
}

export type EnrichedAutolink = [
Expand Down Expand Up @@ -240,7 +242,7 @@ export class Autolinks implements Disposable {
provider != null &&
link.provider?.id === provider.id &&
link.provider?.domain === provider.domain
? provider.getIssueOrPullRequest(id)
? provider.getIssueOrPullRequest(id, link.descriptor)
: undefined,
link,
] satisfies EnrichedAutolink,
Expand Down
13 changes: 10 additions & 3 deletions src/cache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import type { PullRequest } from './git/models/pullRequest';
import type { GitRemote } from './git/models/remote';
import type { RepositoryMetadata } from './git/models/repositoryMetadata';
import type { RemoteProvider } from './git/remotes/remoteProvider';
import type { RichRemoteProvider } from './git/remotes/richRemoteProvider';
import type { RepositoryDescriptor, RichRemoteProvider } from './git/remotes/richRemoteProvider';
import { isPromise } from './system/promise';

type Caches = {
defaultBranch: { key: `repo:${string}`; value: DefaultBranch };
// enrichedAutolinksBySha: { key: `sha:${string}:${string}`; value: Map<string, EnrichedAutolink> };
issuesOrPrsById: { key: `id:${string}:${string}`; value: IssueOrPullRequest };
issuesOrPrsByIdAndRepo: { key: `id:${string}:${string}:${string}`; value: IssueOrPullRequest };
prByBranch: { key: `branch:${string}:${string}`; value: PullRequest };
prsBySha: { key: `sha:${string}:${string}`; value: PullRequest };
repoMetadata: { key: `repo:${string}`; value: RepositoryMetadata };
Expand Down Expand Up @@ -73,11 +74,16 @@ export class CacheProvider implements Disposable {

getIssueOrPullRequest(
id: string,
repo: RepositoryDescriptor | undefined,
remoteOrProvider: RichRemoteProvider | GitRemote<RichRemoteProvider>,
cacheable: Cacheable<IssueOrPullRequest>,
): CacheResult<IssueOrPullRequest> {
const { key, etag } = getRemoteKeyAndEtag(remoteOrProvider);
return this.get('issuesOrPrsById', `id:${id}:${key}`, etag, cacheable);

if (repo == null) {
return this.get('issuesOrPrsById', `id:${id}:${key}`, etag, cacheable);
}
return this.get('issuesOrPrsByIdAndRepo', `id:${id}:${key}:${JSON.stringify(repo)}}`, etag, cacheable);
}

// getEnrichedAutolinks(
Expand Down Expand Up @@ -182,7 +188,8 @@ function getExpiresAt<T extends Cache>(cache: T, value: CacheValue<T> | undefine
case 'defaultBranch':
case 'repoMetadata':
return 0; // Never expires
case 'issuesOrPrsById': {
case 'issuesOrPrsById':
case 'issuesOrPrsByIdAndRepo': {
if (value == null) return 0; // Never expires

// Open issues expire after 1 hour, but closed issues expire after 12 hours unless recently updated and then expire in 1 hour
Expand Down
88 changes: 76 additions & 12 deletions src/git/remotes/github.ts
View file
Original file line number Diff line number Diff line change
@@ -1,27 +1,30 @@
import type { AuthenticationSession, Disposable, QuickInputButton, Range } from 'vscode';
import { env, ThemeIcon, Uri, window } from 'vscode';
import type { Autolink, DynamicAutolinkReference } from '../../annotations/autolinks';
import type { Autolink, DynamicAutolinkReference, MaybeEnrichedAutolink } from '../../annotations/autolinks';
import type { AutolinkReference } from '../../config';
import { GlyphChars } from '../../constants';
import type { Container } from '../../container';
import type {
IntegrationAuthenticationProvider,
IntegrationAuthenticationSessionDescriptor,
} from '../../plus/integrationAuthentication';
import { fromNow } from '../../system/date';
import { log } from '../../system/decorators/log';
import { memoize } from '../../system/decorators/memoize';
import { encodeUrl } from '../../system/encoding';
import { equalsIgnoreCase } from '../../system/string';
import { equalsIgnoreCase, escapeMarkdown } from '../../system/string';
import { supportedInVSCodeVersion } from '../../system/utils';
import type { Account } from '../models/author';
import type { DefaultBranch } from '../models/defaultBranch';
import type { IssueOrPullRequest, SearchedIssue } from '../models/issue';
import { getIssueOrPullRequestMarkdownIcon } from '../models/issue';
import type { PullRequest, PullRequestState, SearchedPullRequest } from '../models/pullRequest';
import { isSha } from '../models/reference';
import type { Repository } from '../models/repository';
import type { RepositoryMetadata } from '../models/repositoryMetadata';
import { ensurePaidPlan, RichRemoteProvider } from './richRemoteProvider';

const autolinkFullIssuesRegex = /\b(?<repo>[^/\s]+\/[^/\s]+)#(?<num>[0-9]+)\b(?!]\()/g;
const autolinkFullIssuesRegex = /\b([^/\s]+\/[^/\s]+?)(?:\\)?#([0-9]+)\b(?!]\()/g;
const fileRegex = /^\/([^/]+)\/([^/]+?)\/blob(.+)$/i;
const rangeRegex = /^L(\d+)(?:-L(\d+))?$/;

Expand All @@ -32,7 +35,14 @@ function isGitHubDotCom(domain: string): boolean {
return equalsIgnoreCase(domain, 'github.com');
}

export class GitHubRemote extends RichRemoteProvider {
type GitHubRepositoryDescriptor =
| {
owner: string;
name: string;
}
| Record<string, never>;

export class GitHubRemote extends RichRemoteProvider<GitHubRepositoryDescriptor> {
@memoize()
protected get authProvider() {
return isGitHubDotCom(this.domain) ? authProvider : enterpriseAuthProvider;
Expand Down Expand Up @@ -77,6 +87,9 @@ export class GitHubRemote extends RichRemoteProvider {
text: string,
outputFormat: 'html' | 'markdown' | 'plaintext',
tokenMapping: Map<string, string>,
enrichedAutolinks?: Map<string, MaybeEnrichedAutolink>,
prs?: Set<string>,
footnotes?: Map<number, string>,
) => {
return outputFormat === 'plaintext'
? text
Expand All @@ -91,28 +104,72 @@ export class GitHubRemote extends RichRemoteProvider {
tokenMapping.set(token, `<a href="${url}" title=${title}>${linkText}</a>`);
}

let footnoteIndex: number;

const issueResult = enrichedAutolinks?.get(num)?.[0];
if (issueResult?.value != null) {
if (issueResult.paused) {
if (footnotes != null && !prs?.has(num)) {
footnoteIndex = footnotes.size + 1;
footnotes.set(
footnoteIndex,
`[${getIssueOrPullRequestMarkdownIcon()} ${
this.name
} Issue or Pull Request ${repo}#${num} $(loading~spin)](${url}${title}")`,
);
}
} else {
const issue = issueResult.value;
const issueTitle = escapeMarkdown(issue.title.trim());
if (footnotes != null && !prs?.has(num)) {
footnoteIndex = footnotes.size + 1;
footnotes.set(
footnoteIndex,
`[${getIssueOrPullRequestMarkdownIcon(
issue,
)} **${issueTitle}**](${url}${title})\\\n${GlyphChars.Space.repeat(
5,
)}${linkText} ${issue.state} ${fromNow(
issue.closedDate ?? issue.date,
)}`,
);
}
}
} else if (footnotes != null && !prs?.has(num)) {
footnoteIndex = footnotes.size + 1;
footnotes.set(
footnoteIndex,
`[${getIssueOrPullRequestMarkdownIcon()} ${
this.name
} Issue or Pull Request ${repo}#${num}](${url}${title})`,
);
}

return token;
});
},
parse: (text: string, autolinks: Map<string, Autolink>) => {
let repo: string;
let ownerAndRepo: string;
let num: string;

let match;
do {
match = autolinkFullIssuesRegex.exec(text);
if (match?.groups == null) break;
if (match == null) break;

({ repo, num } = match.groups);
[, ownerAndRepo, num] = match;

const [owner, repo] = ownerAndRepo.split('/', 2);
autolinks.set(num, {
provider: this,
id: num,
prefix: `${repo}#`,
url: `${this.protocol}://${this.domain}/${repo}/issues/${num}`,
title: `Open Issue or Pull Request #<num> from ${repo} on ${this.name}`,
prefix: `${ownerAndRepo}#`,
url: `${this.protocol}://${this.domain}/${ownerAndRepo}/issues/${num}`,
title: `Open Issue or Pull Request #<num> from ${ownerAndRepo} on ${this.name}`,

description: `${this.name} Issue or Pull Request ${ownerAndRepo}#${num}`,

description: `${this.name} Issue or Pull Request ${repo}#${num}`,
descriptor: { owner: owner, name: repo } satisfies GitHubRepositoryDescriptor,
});
} while (true);
},
Expand Down Expand Up @@ -300,8 +357,15 @@ export class GitHubRemote extends RichRemoteProvider {
protected override async getProviderIssueOrPullRequest(
{ accessToken }: AuthenticationSession,
id: string,
descriptor: GitHubRepositoryDescriptor | undefined,
): Promise<IssueOrPullRequest | undefined> {
const [owner, repo] = this.splitPath();
let owner;
let repo;
if (descriptor != null) {
({ owner, name: repo } = descriptor);
} else {
[owner, repo] = this.splitPath();
}
return (await this.container.github)?.getIssueOrPullRequest(this, accessToken, owner, repo, Number(id), {
baseUrl: this.apiBaseUrl,
});
Expand Down
Loading

0 comments on commit 179b1bf

Please sign in to comment.