Skip to content

Commit 278f698

Browse files
author
vijay upadya
committed
PR feedback updates
1 parent 9059f78 commit 278f698

File tree

3 files changed

+85
-39
lines changed

3 files changed

+85
-39
lines changed

‎src/extension/linkify/common/modelFilePathLinkifier.ts‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -45,29 +45,28 @@ export class ModelFilePathLinkifier implements IContributedLinkifier {
4545
continue;
4646
}
4747

48-
const resolved = await this.resolveTarget(parsed.targetPath, workspaceFolders, parsed.preserveDirectorySlash);
49-
if (!resolved) {
50-
parts.push(original);
51-
continue;
52-
}
48+
// Push promise to resolve in parallel with other matches
49+
parts.push(this.resolveTarget(parsed.targetPath, workspaceFolders, parsed.preserveDirectorySlash).then(resolved => {
50+
if (!resolved) {
51+
return original;
52+
}
5353

54-
const basePath = getWorkspaceFileDisplayPath(this.workspaceService, resolved);
55-
const anchorRange = this.parseAnchor(parsed.anchor);
56-
if (parsed.anchor && !anchorRange) {
57-
parts.push(original);
58-
continue;
59-
}
54+
const basePath = getWorkspaceFileDisplayPath(this.workspaceService, resolved);
55+
const anchorRange = this.parseAnchor(parsed.anchor);
56+
if (parsed.anchor && !anchorRange) {
57+
return original;
58+
}
6059

61-
if (anchorRange) {
62-
const { range, startLine, endLine } = anchorRange;
63-
const displayPath = endLine && startLine !== endLine
64-
? `${basePath}#L${startLine}-${endLine}`
65-
: `${basePath}#L${startLine}`;
66-
parts.push(new LinkifyLocationAnchor(new Location(resolved, range), displayPath));
67-
continue;
68-
}
60+
if (anchorRange) {
61+
const { range, startLine, endLine } = anchorRange;
62+
const displayPath = endLine && startLine !== endLine
63+
? `${basePath}#L${startLine}-L${endLine}`
64+
: `${basePath}#L${startLine}`;
65+
return new LinkifyLocationAnchor(new Location(resolved, range), displayPath);
66+
}
6967

70-
parts.push(new LinkifyLocationAnchor(resolved, basePath));
68+
return new LinkifyLocationAnchor(resolved, basePath);
69+
}));
7170
}
7271

7372
const suffix = text.slice(lastIndex);
@@ -125,9 +124,9 @@ export class ModelFilePathLinkifier implements IContributedLinkifier {
125124
const { text, targetPath, anchor } = parsed;
126125
const textMatchesBase = targetPath === text;
127126
const textIsFilename = !text.includes('/') && targetPath.endsWith(`/${text}`);
128-
const descriptiveAbsolute = this.isAbsolutePath(targetPath) && !!anchor;
127+
const descriptiveWithAnchor = !!anchor; // Allow any descriptive text when anchor is present
129128

130-
return Boolean(workspaceFolders.length) && (textMatchesBase || textIsFilename || descriptiveAbsolute);
129+
return Boolean(workspaceFolders.length) && (textMatchesBase || textIsFilename || descriptiveWithAnchor);
131130
}
132131

133132
private async resolveTarget(targetPath: string, workspaceFolders: readonly Uri[], preserveDirectorySlash: boolean): Promise<Uri | undefined> {
@@ -187,13 +186,14 @@ export class ModelFilePathLinkifier implements IContributedLinkifier {
187186
}
188187

189188
private parseAnchor(anchor: string | undefined): { readonly range: Range; readonly startLine: string; readonly endLine: string | undefined } | undefined {
190-
// Ensure the anchor follows the #L123 or #L123-456 format before parsing it.
191-
if (!anchor || !/^L\d+(?:-\d+)?$/.test(anchor)) {
189+
// Ensure the anchor follows the #L123, #L123-456, or #L123-L456 format before parsing it.
190+
if (!anchor || !/^L\d+(?:-L?\d+)?$/.test(anchor)) {
192191
return undefined;
193192
}
194193

195194
// Capture the start (and optional end) line numbers from the anchor.
196-
const match = /^L(\d+)(?:-(\d+))?$/.exec(anchor);
195+
// Support both L123-456 and L123-L456 formats
196+
const match = /^L(\d+)(?:-L?(\d+))?$/.exec(anchor);
197197
if (!match) {
198198
return undefined;
199199
}

‎src/extension/linkify/test/node/modelFilePathLinkifier.spec.ts‎

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ suite('Model File Path Linkifier', () => {
1414
const result = await linkify(service, '[src/file.ts](src/file.ts#L10-12)');
1515
const anchor = result.parts[0] as LinkifyLocationAnchor;
1616
const expected = new LinkifyLocationAnchor(new Location(workspaceFile('src/file.ts'), new Range(new Position(9, 0), new Position(11, 0))));
17-
expect(anchor.title).toBe('src/file.ts#L10-12');
17+
expect(anchor.title).toBe('src/file.ts#L10-L12');
1818
assertPartsEqual([anchor], [expected]);
1919
});
2020

@@ -45,21 +45,68 @@ suite('Model File Path Linkifier', () => {
4545
assertPartsEqual([anchor], [expected]);
4646
});
4747

48-
test('Should fallback when text does not match base path', async () => {
48+
test('Should fallback when text does not match base path and no anchor', async () => {
4949
const service = createTestLinkifierService('src/file.ts');
50-
const result = await linkify(service, '[other](src/file.ts#L2-3)');
50+
const result = await linkify(service, '[other](src/file.ts)');
5151
assertPartsEqual(result.parts, ['other']);
5252
});
5353

54-
test('Should return label when text omits path', async () => {
54+
test('Should linkify descriptive text with anchor', async () => {
5555
const service = createTestLinkifierService('src/file.ts');
56-
const result = await linkify(service, '[line 54](src/file.ts#L54)');
57-
assertPartsEqual(result.parts, ['line 54']);
56+
const result = await linkify(service, '[Await chat view](src/file.ts#L54)');
57+
const anchor = result.parts[0] as LinkifyLocationAnchor;
58+
const expected = new LinkifyLocationAnchor(new Location(workspaceFile('src/file.ts'), new Range(new Position(53, 0), new Position(53, 0))));
59+
expect(anchor.title).toBe('src/file.ts#L54');
60+
assertPartsEqual([anchor], [expected]);
5861
});
5962

6063
test('Should fallback for invalid anchor syntax', async () => {
6164
const service = createTestLinkifierService('src/file.ts');
6265
const result = await linkify(service, '[src/file.ts](src/file.ts#Lines10-12)');
6366
assertPartsEqual(result.parts, ['src/file.ts']);
6467
});
68+
69+
test('Should handle backticks in link text', async () => {
70+
const service = createTestLinkifierService('file.ts');
71+
const result = await linkify(service, '[`file.ts`](file.ts)');
72+
const anchor = result.parts[0] as LinkifyLocationAnchor;
73+
const expected = new LinkifyLocationAnchor(workspaceFile('file.ts'));
74+
assertPartsEqual([anchor], [expected]);
75+
});
76+
77+
test('Should handle backticks in link text with line anchor', async () => {
78+
const service = createTestLinkifierService('src/file.ts');
79+
const result = await linkify(service, '[`src/file.ts`](src/file.ts#L42)');
80+
const anchor = result.parts[0] as LinkifyLocationAnchor;
81+
const expected = new LinkifyLocationAnchor(new Location(workspaceFile('src/file.ts'), new Range(new Position(41, 0), new Position(41, 0))));
82+
expect(anchor.title).toBe('src/file.ts#L42');
83+
assertPartsEqual([anchor], [expected]);
84+
});
85+
86+
test('Should handle L123-L456 anchor format with L prefix on end line', async () => {
87+
const service = createTestLinkifierService('src/file.ts');
88+
const result = await linkify(service, '[src/file.ts](src/file.ts#L10-L15)');
89+
const anchor = result.parts[0] as LinkifyLocationAnchor;
90+
const expected = new LinkifyLocationAnchor(new Location(workspaceFile('src/file.ts'), new Range(new Position(9, 0), new Position(14, 0))));
91+
expect(anchor.title).toBe('src/file.ts#L10-L15');
92+
assertPartsEqual([anchor], [expected]);
93+
});
94+
95+
test('Should handle descriptive text with L123-L456 anchor format', async () => {
96+
const service = createTestLinkifierService('src/file.ts');
97+
const result = await linkify(service, '[Some descriptive text](src/file.ts#L20-L25)');
98+
const anchor = result.parts[0] as LinkifyLocationAnchor;
99+
const expected = new LinkifyLocationAnchor(new Location(workspaceFile('src/file.ts'), new Range(new Position(19, 0), new Position(24, 0))));
100+
expect(anchor.title).toBe('src/file.ts#L20-L25');
101+
assertPartsEqual([anchor], [expected]);
102+
});
103+
104+
test('Should normalize non-standard L123-456 format to standard L123-L456', async () => {
105+
const service = createTestLinkifierService('src/file.ts');
106+
const result = await linkify(service, '[src/file.ts](src/file.ts#L20-25)');
107+
const anchor = result.parts[0] as LinkifyLocationAnchor;
108+
const expected = new LinkifyLocationAnchor(new Location(workspaceFile('src/file.ts'), new Range(new Position(19, 0), new Position(24, 0))));
109+
expect(anchor.title).toBe('src/file.ts#L20-L25');
110+
assertPartsEqual([anchor], [expected]);
111+
});
65112
});

‎src/extension/prompts/node/agent/fileLinkificationInstructions.tsx‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,13 @@ export class FileLinkificationInstructions extends PromptElement<{}> {
1212
ALWAYS convert file paths to markdown links with 1-based line numbers whenever you cite specific code locations. Use links inside normal sentences, not as the entire answer.<br />
1313
Format examples:<br />
1414
- `The handler lives in [path/to/file.ts](path/to/file.ts#L10).` (single line)<br />
15-
- `See [path/to/file.ts](path/to/file.ts#L10-12) for the range.`<br />
15+
- `See [path/to/file.ts](path/to/file.ts#L10-L12) for the range.`<br />
1616
- `Configuration is defined in [path/to/file.ts](path/to/file.ts).` (whole file)<br />
17-
When you need a bullet list of references, write a short description before the link, for example:<br />
18-
- Await chat view: [path/to/chatQuick.ts](path/to/chatQuick.ts#L142)<br />
19-
- Show widget: [path/to/chatQuick.ts](path/to/chatQuick.ts#L321)<br />
20-
Examples:<br />
21-
❌ `The function is in exampleScript.ts at line 25.`<br />
22-
✓ `The function is in [exampleScript.ts](exampleScript.ts#L25).`<br />
17+
- `The widget renderer attaches anchors ([src/renderer.ts](src/renderer.ts#L42-L48)).` (in parentheses)<br />
18+
When you need a bullet list of references with line numbers, you can use descriptive text:<br />
19+
- [Await chat view](path/to/chatQuick.ts#L142)<br />
20+
- [Show widget](path/to/chatQuick.ts#L321)<br />
21+
NEVER cite file paths as plain text when referring to specific locations. For example, instead of saying `The function is in exampleScript.ts at line 25.`, say `The function is in [exampleScript.ts](exampleScript.ts#L25).`<br />
2322
Critical rules:<br />
2423
- Link text must be the exact file path (no backticks, no `#L` in the visible text, no extra wording). Keep the `#L` anchor in the **link target**, e.g. `[src/file.ts](src/file.ts#L25)`.<br />
2524
- Always include both brackets **and** parentheses. `[src/file.ts](src/file.ts#L25)` is valid; `[src/file.ts#L25]` is not.<br />
@@ -29,7 +28,7 @@ export class FileLinkificationInstructions extends PromptElement<{}> {
2928
- Do not use URIs like file://, vscode://, or https://.<br />
3029
- Percent-encode spaces in target only: `[My File.md](My%20File.md)`<br />
3130
- Each file reference needs complete path (don't abbreviate repeated files)<br />
32-
- Integrate line numbers into anchor: `#L10` or `#L10-12` for ranges<br />
31+
- Integrate line numbers into anchor: `#L10` or `#L10-L12` for ranges<br />
3332
- Don't wrap links in backticks; only cite existing paths from context<br />
3433
</Tag>;
3534
}

0 commit comments

Comments
 (0)