Skip to content

Commit 3baec44

Browse files
drowaudioclaude
andcommitted
Add 2.0.0 changelist, harden --config-base64, add subcommands handoff
- CHANGELIST.md: 2.0.0 entry summarising the CLI11/JSON parser rewrite, --config, the new precedence, and the breaking changes (VERSION not bumped). - --config-base64 (internal parent->child handoff) now errors if combined with any option other than --validate, instead of silently ignoring it. Test added. - SUBCOMMANDS_HANDOFF.md: handoff doc for the deferred subcommand restructure (validate/run-tests/strictness-help with deprecated flat-flag aliases). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a3c9069 commit 3baec44

4 files changed

Lines changed: 143 additions & 1 deletion

File tree

CHANGELIST.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
# pluginval Change List
22

3+
### 2.0.0
4+
- Replaced the hand-rolled command-line parser with CLI11 and a single JSON-based settings pipeline
5+
- Added `--config <file.json>` to load settings from JSON (repeatable; later files win per key)
6+
- Settings precedence is now (lowest to highest): defaults, environment variables, `--config`, command-line options
7+
- Environment variable support is now automatic for every option (including `DISABLED_TESTS`)
8+
- **Breaking:** unrecognised options now produce an error instead of being ignored
9+
- **Breaking:** invalid option values now error instead of silently falling back (e.g. `--rtcheck banana`, `--strictness-level abc`)
10+
- **Breaking:** falsey flag environment variables now mean off (e.g. `SKIP_GUI_TESTS=0` no longer enables skipping)
11+
- `--help` output is now generated by CLI11
12+
313
### 1.0.5
414
- Added static linking to the Windows runtime so it should run on more Windows systems (particularly non-dev machines)
515
- Made `PluginInfoTest` run on the message thread as the functions it calls aren't thread-safe

SUBCOMMANDS_HANDOFF.md

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
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.

Source/CommandLineTests.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,30 @@ struct CommandLineTests : public juce::UnitTest
307307

308308
expect (PluginvalSettings::fromPluginTestOptions (opts2, fileOrID2) == expected);
309309
}
310+
311+
beginTest ("--config-base64 rejects extra options");
312+
{
313+
PluginTests::Options opts;
314+
opts.strictnessLevel = 7;
315+
316+
juce::StringArray childArgs (createCommandLine ("/some/MyPlugin.vst3", opts));
317+
childArgs.remove (0); // drop the executable path
318+
319+
// The legitimate parent -> child handoff parses fine.
320+
{
321+
const auto r = settings_parser::parseTokens (childArgs, emptyEnv());
322+
expect (! r.handled);
323+
expectEquals (r.settings.strictnessLevel, 7);
324+
}
325+
326+
// Combining it with any other option is rejected.
327+
{
328+
childArgs.addArray ({ "--strictness-level", "9" });
329+
const auto r = settings_parser::parseTokens (childArgs, emptyEnv());
330+
expect (r.handled);
331+
expectEquals (r.exitCode, 1);
332+
}
333+
}
310334
}
311335
};
312336

Source/SettingsParser.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,28 @@ Precedence (lowest to highest): defaults, environment variables, --config, comma
316316
ParseResult result;
317317
auto& s = result.settings;
318318

319-
// A base64 JSON handoff from the parent process is fully authoritative.
319+
// --config-base64 carries a fully-resolved settings set from the parent
320+
// process and is authoritative. It is for internal use only and must not be
321+
// combined with other options (which would otherwise be silently ignored);
322+
// the only companion allowed is --validate.
320323
if (const auto b64 = valueForOption (tokens, "--config-base64"); b64.isNotEmpty())
321324
{
325+
for (const auto& token : tokens)
326+
{
327+
if (! token.startsWith ("-") || token == "-")
328+
continue; // a value, not an option
329+
330+
if (const auto name = token.upToFirstOccurrenceOf ("=", false, false);
331+
name != "--config-base64" && name != "--validate")
332+
{
333+
std::cerr << "*** FAILED: --config-base64 is for internal use and cannot be combined "
334+
<< "with other options (got " << name << ")" << std::endl;
335+
result.exitCode = 1;
336+
result.handled = true;
337+
return result;
338+
}
339+
}
340+
322341
s = settings_serializer::fromJsonString (decodeBase64 (b64).toStdString());
323342
s.validatePath = resolvePluginPath (juce::String (s.validatePath)).toStdString();
324343
return result;

0 commit comments

Comments
 (0)