Skip to content

Commit e90fa2c

Browse files
authored
Add Copilot agent skills and expand contributor instructions (#4863)
* improve copilot agent setup: instructions, prompts, and ci environment * add skills for adding commands, diagnostic parsers, settings, backend tests, and handling presets vs kits * clarify Copilot CI environment instructions and improve command registration guidance --------- Co-authored-by: Hannia Valera <hanniavalera@users.noreply.github.com>
1 parent 33531af commit e90fa2c

File tree

7 files changed

+1246
-2
lines changed

7 files changed

+1246
-2
lines changed

.github/copilot-instructions.md

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@ applyTo: "**/*.ts,**/*.tsx,**/package.json,**/*.cmake,**/CMakeLists.txt,**/CMake
1818

1919
## Project conventions
2020

21-
- **Path alias**: `@cmt/*` maps to `src/*` (see `tsconfig.json`). Always use `import foo from '@cmt/foo'` — never relative paths from outside `src/`.
22-
- **Error reporting**: Use `rollbar.invokeAsync()` / `rollbar.invoke()` for top-level error boundaries around event handlers, never bare `try/catch` that silently swallows.
21+
- **Path alias**: `@cmt/*` maps to `src/*` (see `tsconfig.json`). Always use `import foo from '@cmt/foo'` — never relative paths from outside `src/`. `@test/*` maps to `test/*` (defined in both `tsconfig.json` and `test.tsconfig.json`). Use `import ... from '@test/...'` in test files.
22+
- **Error reporting**: Use `rollbar` wrappers for top-level error boundaries around event handlers, never bare `try/catch` that silently swallows.
23+
- `rollbar.invoke(what, fn)` — wraps synchronous code; catches, logs, and re-throws.
24+
- `rollbar.invokeAsync(what, fn)` — wraps async function execution; catches promise rejections.
25+
- `rollbar.takePromise(what, additional, thenable)` — attaches an error handler to an **already-created** Thenable (e.g., a return value from a VS Code API call). Use this when you have a promise but didn't create it via `invokeAsync`.
2326
- **Telemetry**: Use helpers in `src/telemetry.ts` (`logEvent`). Never call the VS Code telemetry API directly.
2427

2528
## Architecture
@@ -58,14 +61,41 @@ Identify the affected layer(s) from the architecture table above. Read the relev
5861
| Cache entries | `CMakeDriver.cmakeCacheEntries` |
5962
| Extension settings | `ConfigurationReader` (`src/config.ts`) — never `vscode.workspace.getConfiguration()` directly |
6063

64+
### Reactive configuration subscriptions
65+
66+
Use `ConfigurationReader.onChange()` for typed, per-setting change notifications:
67+
68+
```typescript
69+
const disposable = config.onChange('settingName', (newValue) => {
70+
// Respond to change
71+
});
72+
// Store disposable in a field or push to a disposables array for cleanup
73+
```
74+
75+
Do **not** use `vscode.workspace.onDidChangeConfiguration` directly — the `onChange()` API is type-safe, fires only for the specific setting, and integrates with the extension's promise tracking.
76+
6177
### Always handle both operating modes
6278

6379
When a code path touches shared logic (configure, build, test, targets, environment), check `CMakeProject.useCMakePresets` and ensure it works correctly in both presets mode and kits/variants mode. Omitting the check for one mode in shared code is a bug waiting to happen. Features that are inherently mode-specific (e.g., kit scanning, preset expansion) are fine to scope to one mode.
6480

81+
### `SpecialKits` sentinel values
82+
83+
Kit names may be sentinel values, not real compiler paths. Always check before treating `kit.name` as a file path:
84+
85+
- `SpecialKits.Unspecified` (`'__unspec__'`) — no kit selected
86+
- `SpecialKits.ScanForKits` (`'__scanforkits__'`) — triggers kit scanning
87+
- `SpecialKits.ScanSpecificDir` (`'__scan_specific_dir__'`) — scan a specific directory
88+
89+
### Kit trust model
90+
91+
Kits have an `isTrusted: boolean` property. Auto-scanned kits from untrusted paths are filtered out before selection and use. This prevents untrusted kits from executing `environmentSetupScript`. Do not bypass this trust filtering when adding kit-related code.
92+
6593
### Always handle both generator types
6694

6795
Single-config uses `CMAKE_BUILD_TYPE`; multi-config uses `--config` at build time. Check the active generator before any build-type logic.
6896

97+
Call `util.isMultiConfGeneratorFast(generator)` before any build-type logic. Multi-config generators include Visual Studio, Xcode, and Ninja Multi-Config. Note: the `cmake.setBuildTypeOnMultiConfig` setting can override multi-config behavior — check it before assuming.
98+
6999
### Localize all user-visible strings
70100

71101
Every file with user-visible text needs the `vscode-nls` boilerplate:
@@ -79,6 +109,14 @@ const localize: nls.LocalizeFunc = nls.loadMessageBundle();
79109
// ❌ bare strings in user-visible output
80110
```
81111

112+
### NLS key naming and `package.nls.json`
113+
114+
Settings: `cmake-tools.configuration.cmake.<settingName>.description` (or `.markdownDescription`)
115+
Commands: `cmake-tools.command.cmake.<commandName>.title`
116+
117+
- Update `package.nls.json` for English strings — **never** modify files under `i18n/` (translations are updated separately).
118+
- When a setting uses `markdownDescription` in `package.json`, the NLS key suffix must be `.markdownDescription`, not `.description`.
119+
82120
### Use the module-scoped logger — never `console.log`
83121

84122
```typescript
@@ -98,10 +136,35 @@ Never concatenate path strings with `/` or `\\`. No exceptions.
98136

99137
`package.json` (`contributes.configuration`), `src/config.ts` (`ConfigurationReader`), and `docs/cmake-settings.md`.
100138

139+
### Setting `scope` in `package.json`
140+
141+
Every setting in `contributes.configuration` must have an explicit `scope`:
142+
- `"resource"` — per-folder; use for anything CMake project-specific (paths, build args, generator, environment).
143+
- `"window"` — global; use for extension-wide UI/behavior settings.
144+
145+
Examples: `cmake.buildDirectory``"resource"` (project-specific path), `cmake.autoSelectActiveFolder``"window"` (global UI behavior).
146+
101147
### Every PR needs a CHANGELOG entry
102148

103149
One entry under the current version in `CHANGELOG.md`, in the appropriate section (`Features:`, `Improvements:`, or `Bug Fixes:`), describing user-visible behavior in the repo's existing tense and category style.
104150

151+
### Adding a new diagnostic parser
152+
153+
1. Create `src/diagnostics/<name>.ts` — extend `RawDiagnosticParser`, implement `handleLine()`
154+
2. Add an instance to the `Compilers` class in `src/diagnostics/build.ts`
155+
3. Add the parser name to `cmake.enabledOutputParsers` enum array **and** default array in `package.json`
156+
4. Add English strings to `package.nls.json` if descriptions change
157+
158+
Currently registered: gcc, gnuld, ghs, diab, msvc, iar, iwyu. Default-enabled: cmake, gcc, gnuld, msvc, ghs, diab.
159+
160+
### Copilot CI environment (agent-only)
161+
162+
> **This section applies to the Copilot coding agent, not human contributors.**
163+
164+
In the Copilot agent CI environment, `.npmrc` is renamed to `.npmrc.bak` and `yarn.lock` registry URLs are patched to use the public npm registry. These are environment artifacts:
165+
- **Never commit** `.npmrc.bak` or the patched `yarn.lock`
166+
- If `git status` shows these as modified, run `git checkout -- .npmrc yarn.lock` before committing
167+
105168
## Testing checklist
106169

107170
- [ ] `yarn unitTests` passes
@@ -111,6 +174,20 @@ One entry under the current version in `CHANGELOG.md`, in the appropriate sectio
111174
- [ ] Behavior verified with **single-config** and **multi-config** generators
112175
- [ ] Windows/macOS/Linux differences considered (paths, env vars, MSVC toolchain, generator availability)
113176

177+
### Backend tests (`yarn backendTests`)
178+
179+
The fastest feedback loop — runs via Mocha with no VS Code instance or display required.
180+
181+
- **Files go in** `test/unit-tests/backend/<name>.test.ts`
182+
- **Framework:** Mocha TDD (`suite`/`test`), Chai `expect` assertions
183+
- **Run:** `yarn backendTests`
184+
- **Mock setup:** `test/unit-tests/backend/setup-vscode-mock.ts` provides minimal `vscode` stubs
185+
186+
**Import strategy:**
187+
1. If the module has **no transitive `vscode` dependency** (e.g., `encodingUtils`, `cmakeValue`) → import directly via `@cmt/*`
188+
2. If the module **transitively imports `vscode`** and the mock is insufficient → **mirror the pure function logic inline** in the test file. This is the established pattern in `expand.test.ts`, `targetMap.test.ts`, `shell-propagation.test.ts`.
189+
3. **Heuristic:** Try `@cmt/*` import first. If it fails due to vscode dependency at runtime, fall back to mirroring.
190+
114191
## Where to start
115192

116193
- **Configure/build/test behavior**`src/cmakeProject.ts` + `src/drivers/`
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
---
2+
name: add-command
3+
description: >
4+
Use when adding a new cmake.* command to CMake Tools. Touches package.json
5+
(contributes.commands), package.nls.json, src/extension.ts (funs array), and
6+
CHANGELOG.md. Triggers: "add command", "register command", "new command palette entry".
7+
---
8+
9+
# Adding a New Command
10+
11+
Recipe for adding a new `cmake.*` command to CMake Tools.
12+
13+
## Files you must touch
14+
15+
| File | What to add |
16+
|------|-------------|
17+
| `package.json` | Command declaration in `contributes.commands` + optional menu entries |
18+
| `package.nls.json` | English title string |
19+
| `src/extension.ts` | Method name in `funs` array + handler method on `ExtensionManager` |
20+
| `CHANGELOG.md` | Entry under the current version |
21+
22+
---
23+
24+
## Step 1 — Declare the command in `package.json`
25+
26+
### 1a — `contributes.commands`
27+
28+
```jsonc
29+
// package.json → contributes.commands
30+
{
31+
"command": "cmake.myCommand",
32+
"title": "%cmake-tools.command.cmake.myCommand.title%",
33+
"category": "CMake"
34+
}
35+
```
36+
37+
### Rules
38+
39+
- **Command ID format:** `cmake.<commandName>` (camelCase).
40+
- **Title:** NLS key in the format `%cmake-tools.command.cmake.<commandName>.title%`.
41+
- **Category:** `"CMake"` — this prefixes the title in the Command Palette as `CMake: <title>`.
42+
- **`when`** (optional): controls when the command appears in the Command Palette.
43+
- **`icon`** (optional): Codicon reference like `"$(settings-gear)"` for tree-view inline buttons.
44+
45+
### 1b — `contributes.menus` (if needed)
46+
47+
Add visibility rules for where the command appears.
48+
49+
**Command Palette visibility:**
50+
51+
```jsonc
52+
// package.json → contributes.menus.commandPalette
53+
{
54+
"command": "cmake.myCommand",
55+
"when": "cmake:enableFullFeatureSet"
56+
}
57+
```
58+
59+
**Sidebar tree-view inline button:**
60+
61+
```jsonc
62+
// package.json → contributes.menus["view/item/context"]
63+
{
64+
"command": "cmake.projectStatus.myCommand",
65+
"when": "view == cmake.projectStatus && cmake:enableFullFeatureSet && viewItem == 'myItem'",
66+
"group": "inline"
67+
}
68+
```
69+
70+
Common `when` clause patterns:
71+
72+
| Pattern | Meaning |
73+
|---------|---------|
74+
| `cmake:enableFullFeatureSet` | Extension is fully activated |
75+
| `useCMakePresets` | Presets mode is active |
76+
| `!useCMakePresets` | Kits/variants mode is active |
77+
| `view == cmake.projectStatus && viewItem == 'kit'` | Specific tree-view item |
78+
| `viewItem =~ /configPreset/` | Regex match on tree-view item |
79+
80+
---
81+
82+
## Step 2 — Add the English string to `package.nls.json`
83+
84+
```json
85+
"cmake-tools.command.cmake.myCommand.title": "My Command Title"
86+
```
87+
88+
For titles containing the product name, use the object form with a translator comment:
89+
90+
```json
91+
"cmake-tools.command.cmake.myCommand.title": {
92+
"message": "Do Something with CMake Tools",
93+
"comment": ["The text 'CMake Tools' should not be localized."]
94+
}
95+
```
96+
97+
> **Do not** modify any file under `i18n/`.
98+
99+
---
100+
101+
## Step 3 — Register the command in `src/extension.ts`
102+
103+
Add the method name to the `funs` array (search for `const funs:` near the end of the file). The `register()` helper
104+
auto-generates the command ID `cmake.<name>`, wraps it with debug logging, and hands
105+
the promise to `rollbar.takePromise()` for error tracking.
106+
107+
```typescript
108+
// src/extension.ts
109+
const funs: (keyof ExtensionManager)[] = [
110+
// ... existing entries ...
111+
'myCommand', // ← add here
112+
];
113+
```
114+
115+
That's it — no manual `registerCommand` call needed. The loop that follows handles registration automatically:
116+
117+
```typescript
118+
for (const key of funs) {
119+
context.subscriptions.push(register(key));
120+
}
121+
```
122+
123+
> Only use manual `vscode.commands.registerCommand()` for commands that need
124+
> custom argument handling (e.g., tree-view context-menu commands that receive
125+
> a node argument). Most commands go through the `funs` array.
126+
127+
---
128+
129+
## Step 4 — Implement the handler on `ExtensionManager`
130+
131+
Add a method to the `ExtensionManager` class in `src/extension.ts`. The method
132+
name must match the string added to the `funs` array.
133+
134+
### Pattern A — Delegate to CMakeProject (most common)
135+
136+
```typescript
137+
myCommand(folder?: vscode.WorkspaceFolder) {
138+
telemetry.logEvent('myCommand');
139+
return this.runCMakeCommand(
140+
cmakeProject => cmakeProject.myCommand(),
141+
folder,
142+
undefined, // precheck (optional)
143+
true // cleanOutputChannel
144+
);
145+
}
146+
```
147+
148+
Then implement the actual logic on `CMakeProject` in `src/cmakeProject.ts`.
149+
150+
### Pattern B — Run for all projects
151+
152+
```typescript
153+
myCommandAll() {
154+
telemetry.logEvent('myCommand', { all: 'true' });
155+
return this.runCMakeCommandForAll(
156+
cmakeProject => cmakeProject.myCommand()
157+
);
158+
}
159+
```
160+
161+
### Pattern C — Direct implementation (no CMakeProject delegation)
162+
163+
```typescript
164+
async myCommand() {
165+
telemetry.logEvent('myCommand');
166+
const result = await vscode.window.showQuickPick(items);
167+
if (!result) {
168+
return;
169+
}
170+
// ... handle result ...
171+
}
172+
```
173+
174+
### Key helpers
175+
176+
| Helper | Use when |
177+
|--------|----------|
178+
| `this.runCMakeCommand(cmd, folder)` | Single-project command |
179+
| `this.runCMakeCommandForAll(cmd)` | Runs on every open CMake project |
180+
| `this.runCMakeCommandForProject(cmd, project)` | Specific project instance |
181+
182+
---
183+
184+
## Step 5 — Add a CHANGELOG entry
185+
186+
Add an entry under the current version in `CHANGELOG.md`, in the `Features:` section.
187+
188+
---
189+
190+
## Verification checklist
191+
192+
- [ ] `package.json` — command declared with NLS title and `"CMake"` category
193+
- [ ] `package.json` — menu entries added (if applicable) with correct `when` clauses
194+
- [ ] `package.nls.json` — English title string added
195+
- [ ] `src/extension.ts` — method name added to `funs` array
196+
- [ ] `src/extension.ts` — handler method implemented on `ExtensionManager`
197+
- [ ] Handler uses `telemetry.logEvent()` for telemetry
198+
- [ ] Handler delegates to `CMakeProject` via `runCMakeCommand` (if project-scoped)
199+
- [ ] `CHANGELOG.md` — entry added
200+
- [ ] `yarn compile` succeeds
201+
- [ ] No files under `i18n/` were modified
202+
203+
---
204+
205+
*See also: [`.github/copilot-instructions.md`](../copilot-instructions.md) for project-wide conventions.*

0 commit comments

Comments
 (0)