Skip to content

Commit a4faabe

Browse files
committed
Address PR feedback: use constants, clarify watcher code, add AGENTS.md
- addService.ts: Use SUPPORTED_LANGUAGES constant instead of hardcoded array - RevealStep.ts: Extract l10n strings to constants before comparison - FileSystemWatcherService.ts: Add clarifying comments for watcher registration flow - AGENTS.md: Add agent development guide per Jeffrey's suggestion
1 parent 813b514 commit a4faabe

File tree

4 files changed

+72
-5
lines changed

4 files changed

+72
-5
lines changed

ext/vscode/AGENTS.md

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# Agent Development Guide
2+
3+
A file for [guiding coding agents](https://agents.md/).
4+
5+
## Commands
6+
7+
- **Install dependencies:** `npm install`
8+
- **Build:** `npm run build`
9+
- **Lint:** `npm run lint`
10+
- **Spell Check:** `npx cspell "src/**/*.ts"`
11+
- **Unit Tests:** `npm run unit-test`
12+
- **Watch mode:** `npm run watch`
13+
- **Package extension:** `npm run package`
14+
15+
## Directory Structure
16+
17+
- Extension entry point: `src/extension.ts`
18+
- Commands: `src/commands/`
19+
- Language features: `src/language/` (IntelliSense, diagnostics, etc.)
20+
- Views & tree providers: `src/views/`
21+
- Utilities: `src/utils/`
22+
- Tests: `src/test/`
23+
- Constants: `src/constants/`
24+
- Services: `src/services/`
25+
26+
## Pre-Commit Checklist
27+
28+
1. Run `npm run lint` and fix all issues
29+
2. Run `npx cspell "src/**/*.ts"` and fix spelling errors
30+
3. Run `npm run unit-test` and ensure all tests pass
31+
4. Update README.md if functionality changed
32+
5. Verify no merge conflict markers in code
33+
34+
## Code Conventions
35+
36+
### Copyright Headers
37+
All TypeScript source files MUST include the Microsoft copyright header:
38+
```typescript
39+
// Copyright (c) Microsoft Corporation. All rights reserved.
40+
// Licensed under the MIT License.
41+
```
42+
43+
### Localization
44+
- All user-facing strings shown in the UI, error messages, etc. must use `vscode.l10n.t()`
45+
- All user-facing strings in package.json must be extracted into package.nls.json
46+
47+
### UI Best Practices
48+
- Instead of `vscode.window.showQuickPick`, use `IActionContext.ui.showQuickPick`
49+
- Instead of `vscode.window.showInputBox`, use `IActionContext.ui.showInputBox`
50+
- Same for `showWarningMessage`, `showOpenDialog`, and `showWorkspaceFolderPick`
51+
52+
### Resource Management
53+
- FileSystemWatchers are a scarce resource on some systems - use the shared `FileSystemWatcherService`
54+
- Dispose resources properly using `vscode.Disposable` pattern
55+
- Use `ExtensionContext.subscriptions` for cleanup
56+
57+
### Testing
58+
- Use Mocha for test framework
59+
- Use Chai for assertions
60+
- Mock VS Code APIs using Sinon
61+
- Keep tests focused and isolated

ext/vscode/src/commands/addService.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import { IActionContext } from '@microsoft/vscode-azext-utils';
55
import * as vscode from 'vscode';
66
import * as yaml from 'yaml';
7+
import { SUPPORTED_LANGUAGES } from '../constants/languages';
78
import { AzureDevCliModel } from '../views/workspace/AzureDevCliModel';
89

910
/**
@@ -50,7 +51,7 @@ export async function addService(context: IActionContext, node?: AzureDevCliMode
5051

5152
// Prompt for programming language
5253
const language = await context.ui.showQuickPick(
53-
['python', 'js', 'ts', 'csharp', 'java', 'go'].map(lang => ({ label: lang })),
54+
SUPPORTED_LANGUAGES.map(lang => ({ label: lang })),
5455
{ placeHolder: vscode.l10n.t('Select programming language') }
5556
);
5657

ext/vscode/src/commands/azureWorkspace/wizard/RevealStep.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,16 @@ export class RevealStep extends AzureWizardExecuteStep<RevealResourceWizardConte
2525

2626
// If reveal failed, show a helpful message with actions
2727
if (result === undefined) {
28+
const copyResourceIdOption = vscode.l10n.t('Copy Resource ID');
29+
const openInPortalOption = vscode.l10n.t('Open in Portal');
2830
const selection = await vscode.window.showInformationMessage(
2931
vscode.l10n.t('Unable to reveal resource in tree. Resource ID: {0}', azureResourceId),
30-
vscode.l10n.t('Copy Resource ID'),
31-
vscode.l10n.t('Open in Portal')
32+
copyResourceIdOption,
33+
openInPortalOption
3234
);
33-
if (selection === vscode.l10n.t('Copy Resource ID')) {
35+
if (selection === copyResourceIdOption) {
3436
await vscode.env.clipboard.writeText(azureResourceId);
35-
} else if (selection === vscode.l10n.t('Open in Portal')) {
37+
} else if (selection === openInPortalOption) {
3638
await vscode.commands.executeCommand('azureResourceGroups.openInPortal', azureResourceId);
3739
}
3840
}

ext/vscode/src/services/FileSystemWatcherService.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export class FileSystemWatcherService implements vscode.Disposable {
2020
let watcherEntry = this.watchers.get(pattern);
2121

2222
if (!watcherEntry) {
23+
// Create new watcher and entry for this pattern
2324
const watcher = vscode.workspace.createFileSystemWatcher(pattern);
2425
watcherEntry = {
2526
watcher,
@@ -28,6 +29,7 @@ export class FileSystemWatcherService implements vscode.Disposable {
2829
this.watchers.set(pattern, watcherEntry);
2930

3031
// Set up forwarding events to all listeners
32+
// Note: watcherEntry is captured in closure and will include all future listeners
3133
watcher.onDidChange(uri => {
3234
watcherEntry!.listeners.forEach(listener => listener(uri));
3335
});
@@ -39,6 +41,7 @@ export class FileSystemWatcherService implements vscode.Disposable {
3941
});
4042
}
4143

44+
// Add callback to listeners (whether entry is new or existing)
4245
watcherEntry.listeners.add(callback);
4346

4447
return {

0 commit comments

Comments
 (0)