Added new macro features. resolve_value + default: key. Added raises to prevent CliGen features from being used outside of a already parsed state, and worked on specs. Will be working on specs later on as well to finish covering regex & redo timeparse & relative_operations

CLAUDE.md

@@ -42,10 +42,19 @@ this project's own test programs.
 Compile-time flow, in order:
 
 1. `Command.inherited` installs a `macro finished` hook on each subclass, which calls
-   `define_command_initializer` (and `def_init` when `@[CommandInfo(def_init: true)]`).
-2. `CliGen::App`'s `macro finished` (in `app/generate.cr`) walks `CliGen::Command.subclasses`
-   and emits `App.generate`, which constructs the runtime `Flag(T)` / `CommandNode(T)` object tree.
-3. At runtime `App.process` lazily calls `generate`, then walks `ARGV`.
+   `validate_command_tree`, `define_command_initializer`, `generate_gather_handler`,
+   `generate_register_command`, `generate_gather_handler` (and `define_singleton_init`
+   when `@[CommandInfo(singleton_init: true)]`).
+2. Each subclass therefore gets its own `self.register_command(array, parent:)` that
+   builds *its* `CommandNode(T)` + `Flag(T)` objects and recurses into its children.
+3. `CliGen::App`'s `macro finished` (in `app/generate.cr`) selects the **root** commands —
+   those whose `@[CommandInfo]` has no `parent:` — and calls `register_command` on each.
+4. At runtime `App.process` lazily calls `generate`, then walks `ARGV`.
+
+Nesting is expressed by **annotation, not inheritance**: a subcommand subclasses
+`CliGen::Command` directly and names its parent via `@[CommandInfo(parent: Other)]`.
+Subclassing a `Command` instead would give it subclasses, which turns `T` into the
+virtual type `T+` and makes `T.has_constant?`/`T.methods` fail with a compiler BUG.
 
 ### Entry point: `src/cligen.cr`
 
@@ -137,12 +146,23 @@ derived as `long.gsub(/--/,"").gsub(/-/,"_").upcase`. Both macros reject an expl
 
 Run at the top of every `process`, so misconfiguration fails on first invocation:
 
-- `Flag#check!` — rejects `-h` / `--help` (`ReservedFlagError`).
+- **Compile time** — `check_flag_vars` validates every declaration (both `argument` and
+  `add_global_flag` route through it); `validate_command_tree` rejects a missing
+  `@[CommandInfo]`, a self-parent, and `parent:` cycles.
+- **Load time** — `add_global_flag` constructs the flag, then checks `GLOBAL_FLAGS` for a
+  `long_key`/`short` collision and `abort`s with `__FILE__:__LINE__` of the call site.
+  Runs during module init, so it is outside `handle_command_raises`.
+- `Flag#check!` — validates `short` against `FLAG_SHORT` and `long_key` against
+  `FLAG_LONG`. Only invoked on `@flags`, never on `GLOBAL_FLAGS`, which is why the
+  built-in `--help`/`--verbose` don't trip their own checks.
 - `CommandNode#check!` — duplicate shorts/longs across `@flags + GLOBAL_FLAGS`
   (`DuplicateFlagError`), duplicate child command names (`DuplicateCommandError`), and
   (when `T != Nil`) requires either subcommands or a `#main` (`MissingDispatchError`).
-- `App#check!` — `super`, then env-var collisions across `all_flags + GLOBAL_FLAGS`.
-  `all_flags` recurses the whole tree; empty env vars are excluded.
+- `App#check!` — `super`, then env-var collisions across `all_flags.uniq + GLOBAL_FLAGS`.
+  `all_flags` recurses the whole tree. **The `.uniq` is load-bearing**: a parent's flag
+  object can appear in several nodes, and `uniq` collapses it via `Reference` identity
+  (`Flag` overrides neither `==` nor `hash`). Adding a custom `==` to `Flag` would
+  silently make this collapse *distinct* flags and stop catching real collisions.
 
 ### Errors: `src/cligen/exceptions.cr`
 
@@ -219,8 +239,8 @@ Called inside a `CliGen::Command` subclass:
 | Macro | File | Purpose |
 |---|---|---|
 | `argument(var : T, description, ...)` | `command/argument.cr` | Declares a flag-backed ivar. Options: `long`, `short`, `validation`, `on_match`, `def_setter`, `def_getter`, `options`, `delimiter`, `format`, `allow_no_verification`, `env_var` |
-| `selection(var : T, description, options, ...)` | `command/selection.cr` | Like `argument` but constrained to a fixed option list |
 | `subcommand(func, description, examples) { ... }` | `command/subcommand.cr` | Defines a `@[SubCommand]` method from a block |
+| `resolve_value(var, default: nil)` | `command/resolve_value.cr` | Reads a flag value from this command **or any ancestor**. Walks the `parent:` chain at compile time to find the declaring class, then walks `handler.parent?` at runtime to find its node. Returns the exact `T`, not a union. If the declaring ivar has no default, `default:` is **required** — the emitted `%default : T = ...` makes Crystal type-check it at the call site, and a `rescue MissingRequiredFlagError` uses it as the fallback |
 | `help_template(filepath)` | `command/help_template.cr` | Per-command ECR override |
 
 Module-level:
@@ -230,7 +250,12 @@ Module-level:
 | `CliGen.add_global_flag(T, long:, description:, ...)` | `global_flag/add_global_flag.cr` | Appends to `GLOBAL_FLAGS`; visible on every command |
 | `CliGen.override_help_template(filepath)` | `cligen.cr` | Project-wide ECR override |
 
-`global_flag.cr` dogfoods `add_global_flag` for the built-in `-v/--verbose`.
+`global_flag.cr` dogfoods `add_global_flag` for the built-in `-v/--verbose` and
+`-h/--help`, both passing `internal: true` to bypass the reserved-name check.
+
+`CliGen::MAX_COMMAND_DEPTH` (default 32, user-overridable by defining it before the
+requires) bounds every parent-chain walk — macros have no `while`, so the walks are
+`{% for i in (1..MAX_COMMAND_DEPTH) %}` with a sentinel. It doubles as the cycle guard.
 
 ### Extension points
 
@@ -244,15 +269,14 @@ in `flag.cr`.
 
 ## Annotations
 
-`src/cligen/annotations.cr` declares seven; only five are wired:
+`src/cligen/annotations.cr` declares six; only four are wired:
 
 | Annotation | Applied to | Status |
 |---|---|---|
-| `@[CommandInfo(description:, def_init:)]` | Command subclass | **Required.** `description` must be a `StringLiteral`; `def_init: true` generates a no-arg initializer + `self.get` singleton accessor |
-| `@[Argument(short:, long:, description:, validation:, on_match:, options:, delimiter:, format:, env_var:)]` | ivar | Emitted by `argument` **and** `selection`; read by `App.generate` and `define_command_initializer` |
+| `@[CommandInfo(description:, parent:, singleton_init:)]` | Command subclass | **Required.** `description` must be a `StringLiteral`. `parent:` names the command this is a subcommand of (see nesting above). `singleton_init: true` generates a no-arg initializer + `self.get` accessor — note the check is `== true`, so any other value silently does nothing |
+| `@[Argument(short:, long:, description:, validation:, on_match:, options:, delimiter:, format:, env_var:)]` | ivar | Emitted by `argument`; read by `App.generate` and `define_command_initializer` |
 | `@[SubCommand(description:, examples:)]` | method | Emitted by `subcommand`; read by `CommandNode#subcommands` and the dispatch `case` |
 | `@[PreRunCommand]` | method | Run unconditionally before subcommand dispatch |
-| `@[Selection]` | ivar | *Read* by `generate`/`define_command_initializer`, but never emitted — `selection` emits `@[Argument]`. Effectively dead. |
 | `@[ProxyCommand]` | — | Declared only; unused |
 | `@[Trigger]` | — | Declared only; unused |
 
@@ -264,9 +288,30 @@ These have each caused real bugs in this codebase — check for them before touc
   `==` silently returns false and `<` raises `undefined macro method 'Path#<'`. Call
   `.resolve` first: `validation.return_type.resolve == Bool`, `type.resolve <= Array`.
   Generic type *parameters* (`T` inside `Flag(T)`) are already `TypeNode` and are safe as-is.
+  This is the single most frequent bug in this codebase — it has appeared five separate
+  times, and it usually fails *silently* (a `Path == TypeNode` comparison is just always
+  false) or with an unrelated-looking error like `undefined macro method 'Path#annotation'`.
+  Resolve once at the point of extraction, not at each use.
+- **`{% cond %}` failing inside a macro reports the wrong line.** When a macro expression
+  raises, the error points at the line that *consumes* the variable, not the assignment
+  that blew up — "undefined macro variable `x`" almost always means the line assigning `x`
+  errored. Look one line up.
+- **Macros cannot be called from macro scope.** `{% if some_macro(x) %}` gives
+  `undefined macro method`. Macros emit code; they don't return values to the evaluator.
+  To share logic, either precompute into a constant both can read, or accept duplication.
+- **`@type.instance_vars` only works in method scope.** In class-body scope (including a
+  `macro finished` directly in a class body) it raises "instance vars are not yet
+  initialized". Declaring an ivar requires class-body scope, so a macro can never read one
+  class's ivars *and* declare ivars in another. Constants are readable in both scopes and
+  are the way around it (`@type.constant("X") << ...` mutates one).
+- **Macro `for` has no `break`**, and reassigning the loop variable does nothing. Bound the
+  loop and guard the body with a sentinel (`{% unless done || found %}`).
+- **Emitted locals leak into the caller's scope.** `flg = ...` in a macro body clobbers a
+  user's `flg`. Use `%flg`, which is unique per expansion (and avoids type unions when the
+  same macro is called with different `T` in one scope).
 - **`{% verbatim do %}`** is required whenever a macro body must emit macro code that runs
-  in the *subclass's* `macro finished` context (see `command.cr`, `def_init.cr`,
-  `define_command_initializer.cr`).
+  in the *subclass's* `macro finished` context (see `command.cr`,
+  `define_singleton_init.cr`, `define_command_initializer.cr`).
 - Signed/unsigned dispatch is done by string inspection, since there's no `UInt` supertype
   to test against: `{% int_case = T.stringify =~ /^UInt/ ? "uint?".id : "int?".id %}`.
 - `macro finished` ordering is why `App.generate` can see every `Command` subclass.
@@ -278,6 +323,17 @@ These have each caused real bugs in this codebase — check for them before touc
   `String`-array `process` overload, then generates most `it` blocks with `{% for int in
   Int.subclasses %}` etc. so every numeric width is covered. `MyGoodData` / `MyBadData`
   exercise `Coercable` / `Parsable`, including the "didn't mark anything processed" failure.
+- `spec/cligen/command_spec.cr` — `resolve_value` at depth 1/2/3, both construction paths
+  (`define_singleton_init`'s no-arg `new` and `new(handler:)`), and the required-flag raise.
+- `spec/cligen/timeparse_spec.cr` / `relative_operation_spec.cr` — the time subsystem.
+- `spec/cligen/regex_spec.cr` — the flag and date matchers.
+
+**Two spec hazards, both already hit:** env vars set in one example leak into others, so
+anything touching `ENV` needs a `before_each { ENV.delete(...) }` — run `crystal spec
+--order=<n>` for a few seeds before trusting a green suite. And time assertions must not
+compare two independently-sampled clocks; `RelativeOperation#apply` takes an explicit
+`Time`, so assert against a fixed `Time.utc` and reserve bracket assertions for
+`Timeparse.parse`'s single `Time.local` call. Check `TZ=UTC crystal spec` too.
 
 `utils/flag_matrix.cr` + `utils/flag_matrix.sh` cover what specs can't: env vars must be
 set before process startup, so each of the 16 cases needs its own process. Run this after
@@ -285,9 +341,14 @@ touching value resolution, `check!`, or help rendering.
 
 ## Current state (branch `object_rework`)
 
-Green: `crystal build --no-codegen` clean, `crystal spec` 164 examples / 0 failures,
-`./utils/flag_matrix.sh` 16/16. Targeting v0.2.0 (`shard.yml` and `CliGen::VERSION` both
-still say `0.1.0`).
+Green: `crystal build --no-codegen` clean, `crystal spec` 276 examples / 0 failures,
+`./utils/flag_matrix.sh` 16/16. `shard.yml` and `CliGen::VERSION` are both bumped to
+`0.2.0`; no git tag exists yet.
+
+**Verifying a change needs a consuming app, not just the lib build.** `crystal build
+src/cligen.cr --no-codegen` does not instantiate `App.process`, `check_for_env_duplicates`,
+or `satisfied?`, so type errors there stay invisible. Write a throwaway `require "cligen"`
+program with a `Command` subclass and build *that*.
 
 Known gaps, all deliberate:
 
@@ -295,9 +356,19 @@ Known gaps, all deliberate:
   DESIGN.md marks it as planned.
 - **`Array(Time)`** — unsupported; the three array element chains have no `Time` case.
 - **Colon-based relative time formats** (`[-+]%H:%M:%S`) — documented in DESIGN.md as planned.
-- `@[Selection]`, `@[ProxyCommand]`, `@[Trigger]` are dead (see above).
-- The `Fiber.yield` log-flush workaround in `app.cr` is fragile; synchronous dispatch
-  would be deterministic.
+- `@[ProxyCommand]` and `@[Trigger]` are declared but unused (see above).
+- The `Fiber.yield` in `app.cr`'s `handle_command_raises` is **load-bearing, not
+  superstition** — Crystal's default `Log` backend dispatches async at INFO even with no
+  `Log.setup` call, so without the yield `abort`'s synchronous stderr write beats the log
+  lines that explain it. A sync dispatcher would be deterministic but is the *consumer's*
+  global setting, so the library can't impose it.
+- `resolve_value` can no longer raise `MissingRequiredFlagError` — the declaring ivar
+  either has a default or the call site must supply one, so both paths are covered. The
+  gate applies only to the parent-chain branch; a local ivar was already resolved during
+  `initialize`, and the not-found branch raises at compile time regardless.
+- `resolve_value`'s own cycle detection is unreachable: `validate_command_tree` runs in
+  `macro finished` and catches cycles before any method body instantiates. The `commands`
+  array it accumulates is still live — it feeds the "valid options are..." error listing.
 
 ## Repo conventions