Skip to content

Commit 715f9b2

Browse files
committed
fix: address Copilot review — add link range assertions & extract settings hint helper
- Add startIndex/length assertions to all 4 underline-wrapped link tests to verify visual and interactive boundaries stay aligned (LOW-1) - Extract private writeSettingsHintLine() to deduplicate the settings hint formatting pattern in DocumentDBShellPty (INFO-1)
1 parent d857a8a commit 715f9b2

2 files changed

Lines changed: 23 additions & 12 deletions

File tree

src/documentdb/shell/DocumentDBShellPty.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -535,10 +535,7 @@ export class DocumentDBShellPty implements vscode.Pseudoterminal {
535535

536536
// Show a hint line and clickable settings link for errors that reference a VS Code setting
537537
if (error instanceof SettingsHintError) {
538-
const settingsLink = this._outputFormatter.formatLinkSentinel(
539-
`${SETTINGS_ACTION_PREFIX}[${error.settingKey}]`,
540-
);
541-
this.writeLine(this._outputFormatter.formatSystemMessage(`${error.settingsHint} ${settingsLink}`));
538+
this.writeSettingsHintLine(error);
542539
}
543540

544541
this._inputHandler.setEnabled(true);
@@ -690,10 +687,7 @@ export class DocumentDBShellPty implements vscode.Pseudoterminal {
690687

691688
// Show a hint line and clickable settings link for errors that reference a VS Code setting
692689
if (error instanceof SettingsHintError) {
693-
const settingsLink = this._outputFormatter.formatLinkSentinel(
694-
`${SETTINGS_ACTION_PREFIX}[${error.settingKey}]`,
695-
);
696-
this.writeLine(this._outputFormatter.formatSystemMessage(`${error.settingsHint} ${settingsLink}`));
690+
this.writeSettingsHintLine(error);
697691
}
698692

699693
// Detect query timeout errors (error code 50: MaxTimeMSExpired / ExceededTimeLimit)
@@ -865,6 +859,14 @@ export class DocumentDBShellPty implements vscode.Pseudoterminal {
865859

866860
// ─── Private: Action line ("Open in Collection View") ────────────────────
867861

862+
/**
863+
* Write a hint line with a clickable settings link for a {@link SettingsHintError}.
864+
*/
865+
private writeSettingsHintLine(error: SettingsHintError): void {
866+
const settingsLink = this._outputFormatter.formatLinkSentinel(`${SETTINGS_ACTION_PREFIX}[${error.settingKey}]`);
867+
this.writeLine(this._outputFormatter.formatSystemMessage(`${error.settingsHint} ${settingsLink}`));
868+
}
869+
868870
/**
869871
* If the result came from a query with a known namespace (db + collection),
870872
* write clickable action lines below the output.

src/documentdb/shell/ShellTerminalLinkProvider.test.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ describe('ShellTerminalLinkProvider', () => {
176176
expect(links).toHaveLength(1);
177177
expect(links[0]).toMatchObject({
178178
linkType: 'collectionView',
179+
startIndex: 0,
180+
length: actionLine.length,
179181
databaseName: 'mydb',
180182
collectionName: 'users',
181183
});
@@ -195,6 +197,8 @@ describe('ShellTerminalLinkProvider', () => {
195197
expect(links).toHaveLength(1);
196198
expect(links[0]).toMatchObject({
197199
linkType: 'collectionView',
200+
startIndex: 0,
201+
length: actionLine.length,
198202
databaseName: 'mydb',
199203
collectionName: 'users',
200204
});
@@ -294,6 +298,8 @@ describe('ShellTerminalLinkProvider', () => {
294298
expect(links).toHaveLength(1);
295299
expect(links[0]).toMatchObject({
296300
linkType: 'settings',
301+
startIndex: 0,
302+
length: actionLine.length,
297303
settingKey: 'documentDB.shell.initTimeout',
298304
});
299305
});
@@ -377,10 +383,9 @@ describe('ShellTerminalLinkProvider', () => {
377383
registerShellTerminal(mockTerminal, () => mockShellInfo('test-cluster-id'));
378384

379385
// Matches real output: gray wraps the whole line, each link segment is underlined separately
380-
const actionLine =
381-
`\x1b[90m\x1b[4m${ACTION_LINE_PREFIX}[mydb.orders]\x1b[24m` +
382-
` ` +
383-
`\x1b[4m${PLAYGROUND_ACTION_PREFIX}[mydb.orders]\x1b[24m\x1b[0m`;
386+
const collectionPart = `\x1b[90m\x1b[4m${ACTION_LINE_PREFIX}[mydb.orders]\x1b[24m`;
387+
const playgroundPart = `\x1b[4m${PLAYGROUND_ACTION_PREFIX}[mydb.orders]\x1b[24m\x1b[0m`;
388+
const actionLine = `${collectionPart} ${playgroundPart}`;
384389
const context = {
385390
terminal: mockTerminal,
386391
line: actionLine,
@@ -390,11 +395,15 @@ describe('ShellTerminalLinkProvider', () => {
390395
expect(links).toHaveLength(2);
391396
expect(links[0]).toMatchObject({
392397
linkType: 'collectionView',
398+
startIndex: 0,
399+
length: collectionPart.length,
393400
databaseName: 'mydb',
394401
collectionName: 'orders',
395402
});
396403
expect(links[1]).toMatchObject({
397404
linkType: 'playground',
405+
startIndex: collectionPart.length + 2, // +2 for " " separator
406+
length: playgroundPart.length,
398407
databaseName: 'mydb',
399408
collectionName: 'orders',
400409
});

0 commit comments

Comments
 (0)