Skip to content

Commit 2f0141e

Browse files
committed
docs: make each repo self-contained for a reader
The repo docs linked to a fleet-level ../AGENTS.md that lives outside every published repository, so on GitHub those links 404 and the standard the code is held to was invisible to anyone reading the code. The standard now lives in this repo's AGENTS.md, and no doc links outside the repository. AGENTS.md also states the commit convention, and CONFIG_DEFAULTS documents why it is exported: nothing imports it but the parity test, and that export is the seam the test needs.
1 parent b6eda1d commit 2f0141e

4 files changed

Lines changed: 173 additions & 43 deletions

File tree

AGENTS.md

Lines changed: 161 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -36,34 +36,160 @@ Conventions: factory functions + `Object.freeze` (no classes), guard clauses, de
3636

3737
## Code style
3838

39-
The full standard, with the reasoning behind each rule, is
40-
[`../AGENTS.md`](../AGENTS.md). It is not optional and it applies to every
41-
change in this repo. In short:
42-
43-
- **Guard clauses first, then the work.** Preconditions return immediately; the
44-
happy path runs at one indent level.
45-
- **No `else`, no `else if`.** Two branches are an early return; many are a
46-
lookup table or a `switch` that returns from every arm.
47-
- **Two levels of nesting, maximum.** A third means the inner block wants to be
48-
its own named function.
49-
- **Truthy checks** (`if (!value)`) — except where `0`, `''` or `false` are
50-
legitimate values, which are tested explicitly. All three have been live bugs
51-
here.
52-
- **Immutable by default:** `readonly` fields, `ReadonlyArray`, `Object.freeze`
53-
on returned objects, never mutate a parameter.
54-
- **Composition, never inheritance.** Factory functions returning frozen
55-
objects; dependencies arrive as a typed bag so tests need no framework.
56-
- **Logic and presentation stay apart.** Extraction, analysis and conversion
57-
return data and never touch `vscode.window.*`; `ui/` renders, `commands/`
58-
orchestrates. A logic module should be testable without the `vscode` mock.
59-
- **Commands are thin** — read config, call logic, hand off to the UI layer,
60-
handle failure.
61-
- **No god files** (~300 lines is the smell), and `types.ts` holds types only.
62-
- **Define it once.** Duplicate regexes and helpers have each shipped as a bug
63-
here, because copies drift and only one copy gets fixed.
64-
- **Complete, descriptive error handling.** Never swallow, never report success
65-
you did not achieve — check what the API returned.
66-
- **Comments explain why, never what.**
39+
These are not preferences to weigh against convenience. They are the shape the
40+
code is expected to take, and a review rejects work that ignores them. The
41+
reason each one exists is stated, because a rule without a reason gets
42+
cargo-culted into places it does not belong.
43+
44+
### Control flow
45+
46+
**Guard clauses first, then the work.** Every function opens with its
47+
preconditions, each one returning immediately. The body that follows is the
48+
happy path at a single indent level, and it reads top to bottom.
49+
50+
```ts
51+
// Yes — preconditions leave, then the real work runs unindented.
52+
function extract(document: TextDocument, config: Configuration): Result {
53+
if (!document) return EMPTY;
54+
if (!isSupported(document.languageId)) return unsupported(document.languageId);
55+
56+
const text = document.getText();
57+
if (!text.trim()) return EMPTY;
58+
59+
return runExtraction(text, config);
60+
}
61+
```
62+
63+
**No `else`. No `else if`.** An `else` is a guard clause that has not been
64+
extracted yet. Two branches become an early return; many branches become a
65+
lookup table or a `switch` that returns from every arm. This is the rule that
66+
does the most work in practice — it is what keeps nesting flat, keeps diffs
67+
small, and stops a function growing a second responsibility inside its own
68+
`else`.
69+
70+
```ts
71+
// No.
72+
if (kind === 'hex') {
73+
return parseHex(value);
74+
} else if (kind === 'rgb') {
75+
return parseRgb(value);
76+
} else {
77+
return null;
78+
}
79+
80+
// Yes — a table. Adding a format touches one line and no control flow.
81+
const PARSERS: Readonly<Record<ColorKind, Parser>> = Object.freeze({
82+
hex: parseHex,
83+
rgb: parseRgb,
84+
hsl: parseHsl,
85+
});
86+
87+
function parse(kind: ColorKind, value: string): Color | null {
88+
const parser = PARSERS[kind];
89+
if (!parser) return null;
90+
return parser(value);
91+
}
92+
```
93+
94+
**Maximum nesting is two levels inside a function.** A third level means the
95+
inner block wants to be its own named function. Loops containing conditionals
96+
containing conditionals are where bugs hide, because no reader holds all three
97+
conditions at once.
98+
99+
**Truthy checks.** `if (!value)` rather than
100+
`if (value === undefined || value === null || value === '')`. The exception is
101+
real and must be respected: when `0`, `''` or `false` are legitimate values,
102+
test explicitly (`value === undefined`, `Number.isFinite(value)`). A threshold
103+
of `0`, an empty string that means "cleared", and `false` from `applyEdit` have
104+
all been live bugs in this family — the terse form is the default, not a
105+
licence to ignore the domain.
106+
107+
### Errors
108+
109+
**Every error path is handled and says something true.** A message names what
110+
failed, why, and what state the user is now in. "Extraction failed" is not a
111+
message; "Could not replace the document contents: the edit was rejected" is.
112+
113+
**Never swallow.** No empty `catch`, no `catch { return null }` that erases a
114+
cause the caller needed, no `|| true`, no `continue-on-error`. If a failure is
115+
genuinely ignorable, the `catch` says why in a comment.
116+
117+
**Failures are values where the caller must react.** A parse failure that the
118+
user should see is reported through the callback or return value the caller
119+
supplied — not thrown past it, and never turned into a silent empty result.
120+
Reserve `throw` for programmer error and for unwinding to a command's outer
121+
handler, which is the one place that decides what the user sees.
122+
123+
**Never report success you did not achieve.** Check what the API returned.
124+
`vscode.workspace.applyEdit` resolves `false` for a read-only document; a
125+
cancelled operation delivers nothing. Announcing a count over work that never
126+
happened is the single most repeated defect in this family's history.
127+
128+
### Data
129+
130+
**Immutable by default.** `readonly` on every interface field, `ReadonlyArray`
131+
on every collection you do not own, `Object.freeze` on returned config and
132+
result objects. Never mutate a parameter. Build a new value and return it.
133+
Where a mutable working copy is genuinely needed, derive the mutable type
134+
(`type Draft<T> = { -readonly [K in keyof T]: T[K] }`) rather than
135+
hand-maintaining a second parallel interface that drifts.
136+
137+
**Composition over inheritance.** Factory functions returning frozen objects,
138+
not classes and not `extends`. Dependencies arrive as a parameter — a typed
139+
deps bag — so a test supplies a fake without a framework. There is no
140+
inheritance hierarchy anywhere in this fleet and there should never be one.
141+
142+
```ts
143+
export function createNotifier(deps: Readonly<{ config: Configuration }>): Notifier {
144+
return Object.freeze({
145+
showInfo: (message: string) => { /* ... */ },
146+
showError: (message: string) => { /* ... */ },
147+
});
148+
}
149+
```
150+
151+
### Structure
152+
153+
**Logic and presentation are separate, always.** Extraction, analysis and
154+
conversion modules compute and return data. They never call
155+
`vscode.window.*`, never format a user-facing sentence, never decide whether a
156+
notification is shown. `ui/` renders; `commands/` orchestrates. The test for
157+
whether you got this right: a logic module should be unit-testable without the
158+
`vscode` mock at all.
159+
160+
**Where a UI framework is involved, the same rule applies to the render.**
161+
Compute above, return markup below. A render body holds no conditionals beyond
162+
a trivial ternary, no data shaping, no derivation — those are named values or
163+
functions above it. Anything else produces JSX no one can read, and it hides
164+
the logic from the tests.
165+
166+
**Commands are thin.** A command reads config, calls logic, hands the result to
167+
the UI layer, and handles failure. When a command file grows a parser or a
168+
formatter, that code belongs in `extraction/` or `ui/`.
169+
170+
**No god files.** Past ~300 lines, a file is doing more than one job and wants
171+
splitting along the seam that is already visible in its exports. `types.ts`
172+
holds types only — no logic, ever.
173+
174+
**Separation of concerns, without ceremony.** One module per real concept, not
175+
one per function. A `utils/` folder of single-line files is as unmaintainable
176+
as a god file; both make you read the whole tree to understand one path.
177+
178+
**Define it once.** Duplicate regexes, duplicate `fullDocumentRange`,
179+
duplicate "is this a supported scheme" checks — each has already shipped as a
180+
bug in this family, because copies drift and only one copy gets fixed. When you
181+
find yourself writing something that exists elsewhere, move it to a shared
182+
module in the same commit.
183+
184+
### Comments
185+
186+
Comments explain **why**, never what. A comment restating the code is noise
187+
that goes stale. A comment recording the reason a non-obvious choice was made —
188+
the constraint, the bug it prevents, the API quirk it works around — is the
189+
most valuable line in the file, and it is what keeps the next person from
190+
"simplifying" it back into a defect.
191+
192+
---
67193

68194
## Invariants (things that were once broken — keep them true)
69195

@@ -107,6 +233,13 @@ The pre-2.0 README carried hand-written test counts and throughput figures that
107233
- **Branch safety:** a `main-safety` ruleset blocks deletion and force-push. Pushes to `main` are otherwise unrestricted by design.
108234
- Secret scanning and push protection are enabled. `VSCE_PAT` and `OVSX_PAT` live in repo secrets and in Doppler (`extensions` / `prd`).
109235

236+
## Commits
237+
238+
Subjects use a conventional prefix — `feat:`, `fix:`, `docs:`, `test:`, `ci:`,
239+
`build:`, `chore:`, `refactor:` — followed by an imperative summary. The body
240+
says why the change was needed and what it prevents; a subject alone is rarely
241+
enough to reconstruct a decision six months later.
242+
110243
## Release
111244

112245
1. Bump `version` in package.json and write the CHANGELOG entry. The entry must describe what actually changed, including bug fixes — it ships inside the VSIX and renders on the listing page.

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6565
### Changed
6666

6767
- Every `else` block is gone (9 of them), replaced by guard clauses and value
68-
expressions, per the fleet standard in `../AGENTS.md`.
68+
expressions, per the code style in `AGENTS.md`.
6969
- Report rendering moved out of `extraction/extract.ts` to `report/format.ts`.
7070
Building the markdown a user reads is presentation and was sitting next to
7171
the detection logic; the two change for different reasons. Extraction drops

CLAUDE.md

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,18 @@
11
# CLAUDE.md
22

3-
[AGENTS.md](AGENTS.md) is the technical source of truth for this repo —
4-
architecture, invariants, toolchain, security automation, release. README.md
5-
is user-facing and partly generated.
6-
7-
**[../AGENTS.md](../AGENTS.md) is the fleet-wide engineering standard**
8-
control flow, error handling, immutability, structure. It governs every change
9-
here; this repo's AGENTS.md covers only what is specific to it. Read it before
10-
writing code.
3+
[AGENTS.md](AGENTS.md) is the technical source of truth for this repo: the
4+
engineering standard the code is held to — control flow, error handling,
5+
immutability, structure — plus this repo's architecture, invariants, toolchain
6+
and release. Read it before writing code. README.md is user-facing and partly
7+
generated.
118

129
## Where to look
1310

1411
| Question | File |
1512
|---|---|
16-
| How should this code be written? | [../AGENTS.md](../AGENTS.md) — the fleet standard, applies to every change here |
17-
| How does this extension work? | [AGENTS.md](AGENTS.md) — architecture, invariants, known limits |
13+
| How should this code be written? | [AGENTS.md](AGENTS.md) — the standard, plus this repo's architecture and invariants |
1814
| What does the user see? | [README.md](README.md) — Testing and Performance are generated |
1915
| What changed? | [CHANGELOG.md](CHANGELOG.md) |
20-
| How do the other nine do it? | [../CLAUDE.md](../CLAUDE.md) — fleet map |
2116

2217
## Gates
2318

src/config/config.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ import * as vscode from 'vscode';
22
import type { Configuration } from '../types';
33

44
/**
5-
* Fallback values, kept identical to the defaults declared in
6-
* package.json contributes.configuration. A unit test asserts parity so
7-
* the two can never drift again.
5+
* The defaults, exported for the parity gate.
6+
*
7+
* Nothing else imports this: `config.test.ts` asserts it matches every
8+
* default declared in package.json, which is the invariant that stops the
9+
* two drifting apart. The export is the seam that test needs.
810
*/
911
export const CONFIG_DEFAULTS = Object.freeze({
1012
copyToClipboardEnabled: false,

0 commit comments

Comments
 (0)