|
| 1 | +# HANDOFF: Restructure pluginval's CLI into subcommands |
| 2 | + |
| 3 | +## Goal |
| 4 | +Turn the current "mode" flags into proper subcommands, each with its own |
| 5 | +argument set, while keeping the old flat flags working as **deprecated aliases |
| 6 | +for one release**: |
| 7 | + |
| 8 | +| New (target) | Old (keep as deprecated alias) | |
| 9 | +|---|---| |
| 10 | +| `pluginval validate [options] <plugin>` (the **default**) | `pluginval --validate <plugin>` / `pluginval <plugin>` | |
| 11 | +| `pluginval run-tests` | `pluginval --run-tests` | |
| 12 | +| `pluginval strictness-help [level]` | `pluginval --strictness-help [level]` | |
| 13 | +| `pluginval --version` / `pluginval --help` | (unchanged) | |
| 14 | + |
| 15 | +All settings options (`--strictness-level`, `--config`, `--rtcheck`, …), |
| 16 | +environment variables, and the JSON precedence pipeline belong to the `validate` |
| 17 | +subcommand and are otherwise unchanged. |
| 18 | + |
| 19 | +This was deliberately deferred from the CLI11 refactor PR (#174). The pipeline |
| 20 | +and `PluginvalSettings` were designed to be reused unchanged. |
| 21 | + |
| 22 | +## Read these first (don't trust this summary — verify against the files) |
| 23 | +- `Source/SettingsParser.cpp/.h` — the parser. Key pieces to reuse: |
| 24 | + - `preprocess()` — deprecation rewrite, macOS flag strip, implicit `--validate`, tokenise. |
| 25 | + - `parseTokens(tokens, env)` — env → `--config` → CLI layering into one `PluginvalSettings`. **This is the `validate` body.** |
| 26 | + - `configureApp(app, s)` — registers every option on a `CLI::App`, used for both the env and CLI passes. |
| 27 | + - `createChildProcessCommandLine()` — the parent→child base64 handoff (`--config-base64 <b64> --validate <path>`). |
| 28 | + - `isCommandLine(tokens)` — what makes `shouldPerformCommandLine` return true. |
| 29 | +- `Source/CommandLine.cpp` — `performCommandLine()` is the dispatcher today: |
| 30 | + token-scans for `--run-tests` / `--strictness-help`, otherwise runs `parseTokens` for validate; `--help`/`--version` go through CLI11. Also `runUnitTests()`, `printStrictnessHelp()`. |
| 31 | +- `Source/Main.cpp` — calls `shouldPerformCommandLine()` then `performCommandLine()` with `getCommandLineParameters()` (a single `juce::String`). |
| 32 | +- `Source/CommandLineTests.cpp` — the test contract. |
| 33 | + |
| 34 | +## Recommended approach: a thin `argv[1]` verb dispatcher (not CLI11 subcommands) |
| 35 | +The existing `parseTokens` pipeline (env/config/CLI layering via two `CLI::App` |
| 36 | +passes) is the hard part and already works. Rather than re-express it inside |
| 37 | +CLI11's native `add_subcommand` machinery (which complicates the env/config |
| 38 | +layering because the options live on the subcommand), peel the verb off the |
| 39 | +front and route: |
| 40 | + |
| 41 | +1. In a new `dispatch()` step (in `SettingsParser` or `CommandLine.cpp`): |
| 42 | + - Tokenise the command line (reuse `preprocess` minus the implicit-validate step, or add a pre-step). |
| 43 | + - Look at the first non-option token: |
| 44 | + - `validate` → strip it, run the existing validate pipeline on the rest. |
| 45 | + - `run-tests` → `runUnitTests()`. |
| 46 | + - `strictness-help` → `printStrictnessHelp(level)`. |
| 47 | + - otherwise → **default to validate** (this preserves `pluginval <plugin>` and `pluginval --strictness-level 5 <plugin>`). |
| 48 | +2. Each verb keeps its own small set of expected args. `validate` reuses |
| 49 | + `configureApp`/`parseTokens` verbatim. `run-tests` takes none. |
| 50 | + `strictness-help` takes an optional level. |
| 51 | + |
| 52 | +This keeps `parseTokens` and the precedence layering untouched — the subcommand |
| 53 | +work is purely a routing layer in front of it. |
| 54 | + |
| 55 | +(If you prefer CLI11-native subcommands instead: put `configureApp` options on a |
| 56 | +`validate` subcommand, and make `buildEnvArgv`/the env pass target that |
| 57 | +subcommand's options. Doable, but more invasive for no functional gain here.) |
| 58 | + |
| 59 | +## Deprecated-alias behaviour (one release) |
| 60 | +Keep the old flat forms working, but print a one-line notice to stderr pointing |
| 61 | +at the new syntax, e.g.: |
| 62 | +- `pluginval --validate x` → run validate; warn `"--validate is deprecated; use 'pluginval validate x'"`. |
| 63 | +- `pluginval --run-tests` → warn `"use 'pluginval run-tests'"`. |
| 64 | +- `pluginval --strictness-help` → warn `"use 'pluginval strictness-help'"`. |
| 65 | +- `pluginval <plugin>` (bare path) → **no warning** (still the documented shorthand for `validate`). |
| 66 | + |
| 67 | +Gate the warnings behind detection of the old flag so the new subcommand form is |
| 68 | +silent. Remove the aliases in the release after next; note it in `CHANGELIST.md`. |
| 69 | + |
| 70 | +## Files to touch |
| 71 | +- `Source/SettingsParser.{h,cpp}` — add the verb dispatch + a `Command` result (validate/run-tests/strictness-help/help/version), or expose a `dispatch()` that returns which verb + the remaining tokens. Keep `parseTokens` as the validate body. |
| 72 | +- `Source/CommandLine.cpp` — `performCommandLine()` routes on the verb; emit the deprecation notices for old flat flags. `shouldPerformCommandLine()` must also recognise the bare verbs (`validate`/`run-tests`/`strictness-help`) in addition to the old flags. |
| 73 | +- `Source/CommandLineTests.cpp` — add tests: each subcommand; default-to-validate; bare-path shorthand; every deprecated alias still works (and warns); `run-tests`/`strictness-help` arg handling. |
| 74 | +- `docs/Command line options.md` — regenerate (`pluginval --help`); CLI11 can show per-subcommand help if you go native, otherwise hand-format the verb list. |
| 75 | +- `CHANGELIST.md` — note the subcommand syntax + the deprecation. |
| 76 | +- `CLAUDE.md` — update the "CLI Settings Pipeline" section to mention the verb layer. |
| 77 | + |
| 78 | +## Gotchas / decisions to make |
| 79 | +- **Child-process handoff.** `createChildProcessCommandLine()` emits `--config-base64 <b64> --validate <path>`. Decide whether the child invocation becomes `validate --config-base64 …` or stays flat. Simplest: keep it flat and have the dispatcher treat a leading `--config-base64`/`--validate` as the (deprecated, unwarned-for-internal) validate path. Note `--config-base64` is now hardened to reject being combined with non-`--validate` options — keep that working under whichever form you choose. |
| 80 | +- **`shouldPerformCommandLine`** is what flips pluginval into CLI (vs GUI) mode in `Main.cpp`. It must return true for `pluginval run-tests` etc., not just the old flags. |
| 81 | +- **`--help` scope.** With the dispatcher, `pluginval --help` is the top-level help (list verbs + the validate options). Consider `pluginval validate --help` for the full option list. CLI11-native subcommands give this for free. |
| 82 | +- **Implicit validate** currently lives in `preprocess`. With an explicit `validate` verb, make sure `pluginval <plugin>` (no verb) still resolves to validate, and `pluginval validate <plugin>` doesn't double-insert `--validate`. |
| 83 | +- **Reconcile** a positional plugin path under `validate` (e.g. `pluginval validate <plugin>`) with the existing `--validate <plugin>` option — pick one canonical form (recommend the positional for the new syntax, mapping it onto `s.validatePath`). |
| 84 | + |
| 85 | +## Verify |
| 86 | +- `pluginval run-tests` passes the full unit suite (it must, it's how CI runs tests). |
| 87 | +- `pluginval validate --strictness-level 10 <plugin>` and `pluginval <plugin>` both validate. |
| 88 | +- Every deprecated alias produces identical behaviour to before (plus a notice). |
| 89 | +- CI matrix green (Linux/macOS/Windows build + dependency). Remember `.github/workflows/build.yaml` uses `--run-tests` and `--strictness-level 10 --validate …`; update those to the new syntax **and** keep an alias test, or the deprecation will fire in CI. |
0 commit comments