diff --git a/CLAUDE.md b/CLAUDE.md index d2ddbbf..355ac0d 100644 --- a/CLAUDE.md +++ b/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=` 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 diff --git a/DESIGN.md b/DESIGN.md index 3f69868..1837215 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -9,7 +9,7 @@ This document outlines the overall design of the CliGen shard & it's underlying ## How it works/High-Level overview -Using crystal macros, you define the shape (arguments/flags, selections, work functions/subcommands, etc) and later on in `src/cligen/app/generate.cr` will use macros to (at compile time) generate `CommandNode(T)` objects & `Flag(T)` objects to contain your command/subcommand/arg parsing code from the data you provided in your `CliGen::Command` subclass. +Using crystal macros, you define the shape (arguments/flags, work functions/subcommands, etc) and later on in `src/cligen/app/generate.cr` will use macros to (at compile time) generate `CommandNode(T)` objects & `Flag(T)` objects to contain your command/subcommand/arg parsing code from the data you provided in your `CliGen::Command` subclass. ## Architecture @@ -72,8 +72,8 @@ The examples like above provide a "DSL-esk" way of defining: - Instance Variables, - Short/Long flags - Description of the flags (used in the help output as well) -- A verification proc/lambda for doing ad-hoc checks of the value provided by the user (essentially allowing you to implement your own option: key like in selection) -- Selections (currently compile-time and will open it up to runtime collecting of options later on based on defined annotations in the class) +- A verification proc/lambda for doing ad-hoc checks of the value provided by the user (for cases where the `options:` key isn't expressive enough) +- A static list of valid options via the `options:` key (currently compile-time; will open up to runtime collection of options later on) - define subcommands of this current command @@ -405,7 +405,7 @@ This object serves as a wrapper around ARGV objects/strings/items and is used to ### Markdown Documentation Generation -Since all command metadata is present in annotations at compile time (`@[CliGen::CommandInfo]`, `@[CliGen::SubCommand]`, `@[CliGen::Argument]`, `@[CliGen::Selection]`), the framework can walk the same structures that `generate.cr` already walks and render them into a Markdown document instead of a `CommandNode` tree. +Since all command metadata is present in annotations at compile time (`@[CliGen::CommandInfo]`, `@[CliGen::SubCommand]`, `@[CliGen::Argument]`), the framework can walk the same structures that `generate.cr` already walks and render them into a Markdown document instead of a `CommandNode` tree. The generation would be driven by a `macro finished` block (similar to `generate.cr`) that emits a `self.generate_docs` class method on `App`. This method walks every `Command` subclass and its annotations to produce a structured document. @@ -466,7 +466,7 @@ complete -F _myapp myapp Implementation notes: * Script body generated at compile time via a `macro finished` walk of `Command.subclasses` -* `@[CliGen::Selection]` options (`%w[json yaml ecr]`) can be included as valid completions for their flag +* `@[CliGen::Argument]` options (`%w[json yaml ecr]`) can be included as valid completions for their flag * Install path: `myapp --generate-completion bash > ~/.bash_completion.d/myapp` or printed with instructions * Same annotation data used by the doc generator, so both stay in sync with the command definition diff --git a/README.md b/README.md index 2898ea7..3dcd4ed 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ A Crystal shard that generates CLI parsers from class definitions using annotati ## How It Works -Subclass `CliGen::Command`, annotate your instance variables with `@[CliGen::Argument]` or `@[CliGen::Selection]`, and register the command with an `CliGen::App`. At compile time, macros inspect the annotations and generate typed `Flag(T)` objects; at runtime, `CliGen::App.process` walks the `CommandNode` tree to route arguments, populate your command instance, and dispatch to the right method. +Subclass `CliGen::Command`, annotate your instance variables with `@[CliGen::Argument]`, and register the command with an `CliGen::App`. At compile time, macros inspect the annotations and generate typed `Flag(T)` objects; at runtime, `CliGen::App.process` walks the `CommandNode` tree to route arguments, populate your command instance, and dispatch to the right method. ## Installation diff --git a/docs/404.html b/docs/404.html index a50a38a..70d069b 100644 --- a/docs/404.html +++ b/docs/404.html @@ -117,6 +117,11 @@ +
  • + Common + +
    • +
  • ConfigurationError @@ -182,6 +187,11 @@
    • +
  • + InternalVar + +
    • +
  • InvalidFlagValueError @@ -252,11 +262,6 @@
    • -
  • - Selection - -
    • -
  • SubCommand diff --git a/docs/CliGen.html b/docs/CliGen.html index 46f3b70..251e2c9 100644 --- a/docs/CliGen.html +++ b/docs/CliGen.html @@ -117,6 +117,11 @@
    • +
  • + Common + +
    • +
  • ConfigurationError @@ -182,6 +187,11 @@
    • +
  • + InternalVar + +
    • +
  • InvalidFlagValueError @@ -252,11 +262,6 @@
    • -
  • - Selection - -
    • -
  • SubCommand @@ -396,12 +401,22 @@
    - cligen/command/def_init.cr + cligen/command/define_command_initializer.cr
    - cligen/command/define_command_initializer.cr + cligen/command/define_singleton_init.cr + +
    + + + cligen/command/generate_gather_handler.cr + +
    + + + cligen/command/generate_register_command.cr
    @@ -411,7 +426,7 @@
    - cligen/command/selection.cr + cligen/command/resolve_value.cr
    @@ -421,6 +436,11 @@
    + cligen/command/validate_command_tree.cr + +
    + + cligen/command_node.cr
    @@ -500,8 +520,33 @@ +
    + MAX_COMMAND_DEPTH = 32 +
    + +
    +

    +CliGen::MAX_COMMAND_DEPTH

    +

    This exists to prevent the user from defining a command tree +that extends past the compile-time configured max via the +CliGen::MAX_COMMAND_DEPTH constant.

    +

    The reason this is a thing is because crystal macros don't allow for +unbounded while's/until's in macros, meaning it always has to be +deterministic. SO to deal with this and still allow for subcommand +defining you need either go with the default (32 command depth) or +define your own larger max (understand this will affect compile-time +due to this directly affecting loops in the Command macros).

    +

    So to still support this I had to make bounded for-loops usng

    +
    {% for i in (1..CliGen::MAX_COMMAND_DEPTH) %}
+  ...do checks...
+{% end }
    +
    + +
    - VERSION = "0.1.0" + VERSION = "0.2.0"
    @@ -525,7 +570,9 @@