Skip to content

Commit 854b78e

Browse files
alexr00alexr00
committed
nits and generated tests
1 parent 7dfdba8 commit 854b78e

4 files changed

Lines changed: 197 additions & 10 deletions

File tree

‎src/github/issueOverview.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -814,8 +814,7 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
814814
new vscode.Position(startLine - 1, 0),
815815
new vscode.Position(endLine - 1, Number.MAX_SAFE_INTEGER)
816816
);
817-
const document = await vscode.workspace.openTextDocument(fileUri);
818-
await vscode.window.showTextDocument(document, {
817+
await vscode.window.showTextDocument(fileUri, {
819818
selection,
820819
viewColumn: vscode.ViewColumn.One
821820
});

‎src/github/pullRequestOverview.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import * as crypto from 'crypto';
88
import * as vscode from 'vscode';
9-
import { OpenCommitChangesArgs } from '../../common/views';
9+
import { OpenCommitChangesArgs, OpenLocalFileArgs } from '../../common/views';
1010
import { openPullRequestOnGitHub } from '../commands';
1111
import { getCopilotApi } from './copilotApi';
1212
import { SessionIdForPr } from './copilotRemoteAgent';
@@ -674,7 +674,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
674674
}
675675
}
676676

677-
private async openDiffFromLink(message: IRequestMessage<{ file: string; startLine: number; endLine: number; href: string }>): Promise<void> {
677+
private async openDiffFromLink(message: IRequestMessage<OpenLocalFileArgs>): Promise<void> {
678678
try {
679679
const { file, startLine } = message.args;
680680
const fileChanges = await this._item.getFileChangesInfo();

‎src/test/common/utils.test.ts‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,188 @@ describe('utils', () => {
5151
assert.strictEqual(utils.formatError(error), 'Cannot push to this repo');
5252
});
5353
});
54+
55+
describe('processPermalinks', () => {
56+
const repoOwner = 'microsoft';
57+
const repoName = 'vscode';
58+
const authority = 'github.com';
59+
const sha = 'a'.repeat(40);
60+
61+
function makePermalink(filePath: string, startLine: number, endLine?: number): string {
62+
const lineRef = endLine ? `#L${startLine}-L${endLine}` : `#L${startLine}`;
63+
return `<a href="https://github.com/microsoft/vscode/blob/${sha}/${filePath}${lineRef}">link text</a>`;
64+
}
65+
66+
it('should add data attributes when file exists locally', async () => {
67+
const html = makePermalink('src/file.ts', 10);
68+
const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true);
69+
70+
assert(result.includes('data-local-file="src/file.ts"'));
71+
assert(result.includes('data-start-line="10"'));
72+
assert(result.includes('data-end-line="10"'));
73+
assert(result.includes('data-link-type="blob"'));
74+
assert(result.includes('data-permalink-processed="true"'));
75+
assert(result.includes('view on GitHub'));
76+
});
77+
78+
it('should set end line when range is specified', async () => {
79+
const html = makePermalink('src/file.ts', 10, 20);
80+
const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true);
81+
82+
assert(result.includes('data-start-line="10"'));
83+
assert(result.includes('data-end-line="20"'));
84+
});
85+
86+
it('should not modify links when file does not exist locally', async () => {
87+
const html = makePermalink('src/file.ts', 10);
88+
const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => false);
89+
90+
assert.strictEqual(result, html);
91+
});
92+
93+
it('should not modify non-permalink links', async () => {
94+
const html = '<a href="https://example.com">example</a>';
95+
const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true);
96+
97+
assert.strictEqual(result, html);
98+
});
99+
100+
it('should not modify links to a different repo', async () => {
101+
const html = `<a href="https://github.com/other/repo/blob/${sha}/src/file.ts#L10">link</a>`;
102+
const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true);
103+
104+
assert.strictEqual(result, html);
105+
});
106+
107+
it('should skip already processed links', async () => {
108+
const html = `<a data-permalink-processed="true" href="https://github.com/microsoft/vscode/blob/${sha}/src/file.ts#L10">link</a>`;
109+
const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true);
110+
111+
assert.strictEqual(result, html);
112+
});
113+
114+
it('should process multiple links independently', async () => {
115+
const html = makePermalink('src/exists.ts', 1) + makePermalink('src/missing.ts', 2);
116+
const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async (path) => path === 'src/exists.ts');
117+
118+
assert(result.includes('data-local-file="src/exists.ts"'));
119+
assert(!result.includes('data-local-file="src/missing.ts"'));
120+
});
121+
122+
it('should return original HTML when fileExistsCheck throws', async () => {
123+
const html = makePermalink('src/file.ts', 10);
124+
const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => { throw new Error('fail'); });
125+
126+
assert.strictEqual(result, html);
127+
});
128+
129+
it('should handle links without surrounding text', async () => {
130+
const html = makePermalink('src/file.ts', 5);
131+
const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true);
132+
133+
assert(result.includes('link text'));
134+
assert(result.includes('data-local-file="src/file.ts"'));
135+
});
136+
});
137+
138+
describe('processDiffLinks', () => {
139+
const repoOwner = 'microsoft';
140+
const repoName = 'vscode';
141+
const authority = 'github.com';
142+
const prNumber = 123;
143+
const diffHash = 'a'.repeat(64);
144+
145+
function makeDiffLink(hash: string, startLine?: number, endLine?: number, variant: 'files' | 'changes' = 'files'): string {
146+
let fragment = `diff-${hash}`;
147+
if (startLine !== undefined) {
148+
fragment += `R${startLine}`;
149+
if (endLine !== undefined) {
150+
fragment += `-R${endLine}`;
151+
}
152+
}
153+
return `<a href="https://github.com/microsoft/vscode/pull/${prNumber}/${variant}#${fragment}">link text</a>`;
154+
}
155+
156+
it('should add data attributes when hash maps to a file', async () => {
157+
const hashMap: Record<string, string> = { [diffHash]: 'src/file.ts' };
158+
const html = makeDiffLink(diffHash, 10);
159+
const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber);
160+
161+
assert(result.includes('data-local-file="src/file.ts"'));
162+
assert(result.includes('data-start-line="10"'));
163+
assert(result.includes('data-end-line="10"'));
164+
assert(result.includes('data-link-type="diff"'));
165+
assert(result.includes('data-permalink-processed="true"'));
166+
assert(result.includes('view on GitHub'));
167+
});
168+
169+
it('should set end line when range is specified', async () => {
170+
const hashMap: Record<string, string> = { [diffHash]: 'src/file.ts' };
171+
const html = makeDiffLink(diffHash, 10, 20);
172+
const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber);
173+
174+
assert(result.includes('data-start-line="10"'));
175+
assert(result.includes('data-end-line="20"'));
176+
});
177+
178+
it('should default start line to 1 when no line is specified', async () => {
179+
const hashMap: Record<string, string> = { [diffHash]: 'src/file.ts' };
180+
const html = makeDiffLink(diffHash);
181+
const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber);
182+
183+
assert(result.includes('data-start-line="1"'));
184+
assert(result.includes('data-end-line="1"'));
185+
});
186+
187+
it('should not modify links when hash is not in the map', async () => {
188+
const hashMap: Record<string, string> = {};
189+
const html = makeDiffLink(diffHash, 10);
190+
const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber);
191+
192+
assert.strictEqual(result, html);
193+
});
194+
195+
it('should not modify non-diff links', async () => {
196+
const hashMap: Record<string, string> = { [diffHash]: 'src/file.ts' };
197+
const html = '<a href="https://example.com">example</a>';
198+
const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber);
199+
200+
assert.strictEqual(result, html);
201+
});
202+
203+
it('should not modify links to a different repo', async () => {
204+
const hashMap: Record<string, string> = { [diffHash]: 'src/file.ts' };
205+
const html = `<a href="https://github.com/other/repo/pull/${prNumber}/files#diff-${diffHash}R10">link</a>`;
206+
const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber);
207+
208+
assert.strictEqual(result, html);
209+
});
210+
211+
it('should skip already processed links', async () => {
212+
const hashMap: Record<string, string> = { [diffHash]: 'src/file.ts' };
213+
const html = `<a data-permalink-processed="true" href="https://github.com/microsoft/vscode/pull/${prNumber}/files#diff-${diffHash}R10">link</a>`;
214+
const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber);
215+
216+
assert.strictEqual(result, html);
217+
});
218+
219+
it('should match links using changes variant', async () => {
220+
const hashMap: Record<string, string> = { [diffHash]: 'src/file.ts' };
221+
const html = makeDiffLink(diffHash, 5, undefined, 'changes');
222+
const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber);
223+
224+
assert(result.includes('data-local-file="src/file.ts"'));
225+
assert(result.includes('data-start-line="5"'));
226+
});
227+
228+
it('should process multiple links independently', async () => {
229+
const otherHash = 'b'.repeat(64);
230+
const hashMap: Record<string, string> = { [diffHash]: 'src/found.ts' };
231+
const html = makeDiffLink(diffHash, 1) + makeDiffLink(otherHash, 2);
232+
const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber);
233+
234+
assert(result.includes('data-local-file="src/found.ts"'));
235+
assert(!result.includes('data-local-file="src/other.ts"'));
236+
});
237+
});
54238
});

‎webviews/common/context.tsx‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { createContext } from 'react';
77
import { getState, setState, updateState } from './cache';
88
import { COMMENT_TEXTAREA_ID } from './constants';
99
import { getMessageHandler, MessageHandler } from './message';
10-
import { CloseResult, DescriptionResult, OpenCommitChangesArgs } from '../../common/views';
10+
import { CloseResult, DescriptionResult, OpenCommitChangesArgs, OpenLocalFileArgs } from '../../common/views';
1111
import { IComment } from '../../src/common/comment';
1212
import { EventType, ReviewEvent, SessionLinkInfo, TimelineEvent } from '../../src/common/timelineEvent';
1313
import { IProjectItem, MergeMethod, PullRequestCheckStatus, ReadyForReview } from '../../src/github/interface';
@@ -361,12 +361,16 @@ export class PRContext {
361361

362362
public openSessionLog = (link: SessionLinkInfo) => this.postMessage({ command: 'pr.open-session-log', args: { link } });
363363

364-
public openLocalFile = (file: string, startLine: number, endLine: number, href: string) =>
365-
this.postMessage({ command: 'pr.open-local-file', args: { file, startLine, endLine, href } });
366-
367-
public openDiffFromLink = (file: string, startLine: number, endLine: number, href: string) =>
368-
this.postMessage({ command: 'pr.open-diff-from-link', args: { file, startLine, endLine, href } });
364+
public openLocalFile = (file: string, startLine: number, endLine: number, href: string) => {
365+
const args: OpenLocalFileArgs = { file, startLine, endLine, href };
366+
this.postMessage({ command: 'pr.open-local-file', args });
367+
};
369368

369+
public openDiffFromLink = (file: string, startLine: number, endLine: number, href: string) => {
370+
const args: OpenLocalFileArgs = { file, startLine, endLine, href };
371+
this.postMessage({ command: 'pr.open-diff-from-link', args });
372+
};
373+
370374
public viewCheckLogs = (status: PullRequestCheckStatus) => this.postMessage({ command: 'pr.view-check-logs', args: { status } });
371375

372376
public openCommitChanges = async (commitSha: string) => {

0 commit comments

Comments
 (0)