Skip to content

Commit 925d9b5

Browse files
committed
improve recursive reference error
1 parent d2c2a22 commit 925d9b5

File tree

1 file changed

+30
-16
lines changed

1 file changed

+30
-16
lines changed

src/vs/workbench/contrib/chat/common/promptFileReferenceErrors.ts

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,9 @@ export class RecursiveReference extends ResolveError {
115115
public override errorType = 'RecursiveReferenceError';
116116

117117
/**
118-
* Default string representation of the recursive path.
118+
* Cached default string representation of the recursive path.
119119
*/
120-
public readonly recursivePathString: string;
120+
private defaultPathStringCache: string | undefined;
121121

122122
constructor(
123123
uri: URI,
@@ -130,29 +130,30 @@ export class RecursiveReference extends ResolveError {
130130
`Recursive path must contain at least two paths, got '${recursivePath.length}'.`,
131131
);
132132

133-
const pathString = recursivePath.join(DEFAULT_RECURSIVE_PATH_JOIN_CHAR);
134133
super(
135-
uri,
136-
`Recursive references found: ${pathString}.`,
134+
uri, 'Recursive references found.',
137135
);
138136
}
139137

138+
public override get message(): string {
139+
return `${super.message} ${this.getRecursivePathString('fullpath')}`;
140+
}
141+
140142
/**
141143
* Returns a string representation of the recursive path.
142144
*/
143145
public getRecursivePathString(
144146
filename: 'basename' | 'fullpath',
145147
pathJoinCharacter: string = DEFAULT_RECURSIVE_PATH_JOIN_CHAR,
146148
): string {
147-
/**
148-
* TODO: @lego - this not currently true though
149-
* TODO: @lego - cache
150-
*/
151-
if (filename === 'fullpath' && pathJoinCharacter === DEFAULT_RECURSIVE_PATH_JOIN_CHAR) {
152-
return this.recursivePathString;
149+
const isDefault = (filename === 'fullpath') &&
150+
(pathJoinCharacter === DEFAULT_RECURSIVE_PATH_JOIN_CHAR);
151+
152+
if (isDefault && (this.defaultPathStringCache !== undefined)) {
153+
return this.defaultPathStringCache;
153154
}
154155

155-
return this.recursivePath
156+
const result = this.recursivePath
156157
.map((path) => {
157158
if (filename === 'fullpath') {
158159
return `'${path}'`;
@@ -168,6 +169,12 @@ export class RecursiveReference extends ResolveError {
168169
);
169170
})
170171
.join(pathJoinCharacter);
172+
173+
if (isDefault) {
174+
this.defaultPathStringCache = result;
175+
}
176+
177+
return result;
171178
}
172179

173180
/**
@@ -183,15 +190,22 @@ export class RecursiveReference extends ResolveError {
183190
return false;
184191
}
185192

186-
// TODO: @lego - check array lengths first
193+
// performance optimization - compare number of paths in the
194+
// recursive path chains first to avoid comparison of all strings
195+
if (this.recursivePath.length !== other.recursivePath.length) {
196+
return false;
197+
}
198+
199+
const myRecursivePath = this.getRecursivePathString('fullpath');
200+
const theirRecursivePath = other.getRecursivePathString('fullpath');
187201

188-
// performance optimization - if the paths lengths don't match,
202+
// performance optimization - if the path lengths don't match,
189203
// no need to compare entire strings as they must be different
190-
if (this.recursivePathString.length !== other.recursivePathString.length) {
204+
if (myRecursivePath.length !== theirRecursivePath.length) {
191205
return false;
192206
}
193207

194-
return this.recursivePathString === other.recursivePathString;
208+
return myRecursivePath === theirRecursivePath;
195209
}
196210

197211
/**

0 commit comments

Comments
 (0)