Refactor CLI parsing onto a single settings struct (CLI11 + nlohmann/json)#174
Closed
drowaudio wants to merge 12 commits into
Closed
Refactor CLI parsing onto a single settings struct (CLI11 + nlohmann/json)#174drowaudio wants to merge 12 commits into
drowaudio wants to merge 12 commits into
Conversation
Prepares for the CLI parser refactor onto magic_args + a JSON-merge settings pipeline. - target_compile_features: cxx_std_20 -> cxx_std_23 (magic_args requires C++23) - CPMAddPackage magic_args v0.2.1 (header-only INTERFACE: magic_args::magic_args) - CPMAddPackage nlohmann/json 3.12.0 (nlohmann_json::nlohmann_json) - link both into the pluginval target Verified: clean build + link under C++23 with juce_recommended_warning_flags on JUCE 8.0.13 (macOS/AppleClang); binary runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduces a single parser-agnostic settings struct and a layered
JSON pipeline that replaces the bespoke juce::ArgumentList parser.
Pipeline: each input layer (--config file, environment, CLI) becomes a
sparse nlohmann::json object; layers are merged with merge_patch in
precedence order (defaults < config < env < CLI, CLI wins) and
deserialised in one step into PluginvalSettings, which converts to the
existing PluginTests::Options at the JUCE boundary.
- PluginvalSettings.h: unified std-typed struct + toPluginTestOptions()
+ fromPluginTestOptions(); NLOHMANN_DEFINE_TYPE..._WITH_DEFAULT so
missing keys fall back to defaults; RealtimeCheck enum<->string mapping.
- SettingsSerializer.{h,cpp}: JSON load/save + the comma-list, hex-seed
and disabled-tests(file-or-list) coercions shared by the CLI/env layers.
- SettingsParser.{h,cpp}: preprocess (deprecation rewrite, macOS flag
strip, implicit --validate), magic_args parse of the flat options into
the sparse CLI layer, env layer, merge, and the child-process handoff.
- Child process round-trip now uses an authoritative base64 JSON arg
(--config-base64) instead of per-flag re-serialisation, avoiding
quote/tokenisation hazards. Round-trip asserted in tests.
- CommandLine.{h,cpp}: thin adapter; old manual parsers/env-merge/help
text deleted. magic_args drives --help (auto usage + env-var trailer).
- New --config <file.json> option; precedence covered by tests.
- CommandLineTests.cpp rewritten against the new pipeline (defaults,
parser, hex seed, comma lists, rtcheck, path resolution, implicit
validate, env precedence, config precedence, round-trip).
- CMake: macOS deployment target 10.11 -> 13.3 (std::format in magic_args
requires libc++ floating-point to_chars, available on 13.3+); add new
sources.
All --run-tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- docs/Command line options.md regenerated from magic_args --help output - CLAUDE.md: document the JSON-merge settings pipeline, new source files, magic_args + nlohmann/json deps, C++23, and the macOS 13.3 deployment target Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses review feedback that the magic_args version duplicated the
settings into a parallel PluginvalCliArgs struct plus manual CLI->JSON
and ENV->JSON bridges (5-6 edit sites per new option), and forced
cxx_std_23 + a macOS 13.3 deployment target (std::format).
CLI11 binds each option directly to a PluginvalSettings member, with the
environment variable on the same line via ->envname(). Adding an option
is now three edits: a struct member, a nlohmann macro entry, and one
add_option(...) line. No parallel struct, no cliArgsToJson, no envLayer.
- SettingsParser: rewritten on CLI11; comma lists via ->delimiter(','),
enum via CheckedTransformer, hex/int seed via a small callback. --config
seeds the struct before parse so precedence stays defaults<config<env<CLI.
--help/--version handled by CLI11 (auto usage + footer).
- SettingsSerializer: dropped the comma/JSON-array coercions; kept the hex
seed + disabled-tests(file-or-list) helpers and JSON load/save.
- Child-process base64 JSON handoff (--config-base64) unchanged.
- CommandLineTests: env tests now use setenv/unsetenv (CLI11 reads the real
environment); all other cases unchanged. All --run-tests pass.
- CMake: magic_args -> CLI11 2.6.2; revert cxx_std_23 -> cxx_std_20 and
CMAKE_OSX_DEPLOYMENT_TARGET 13.3 -> 10.11.
- Docs/CLAUDE.md updated.
Verified: full build (validator ON, C++20, macOS 10.11), --run-tests pass,
real --validate SUCCESS, --config precedence covered by tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
drowaudio
changed the title
Three regressions surfaced by CI (develop is green): - Linux build: std::int64_t (== long on LP64) vs juce::int64 (== long long) made expectEquals template deduction fail. Cast the settings int64 fields to juce::int64 in CommandLineTests. (macOS compiled because there the two types are identical.) - "Pluginval as Dependency" Verify: verify_pluginval runs `pluginval --help | grep "JUCE v<version>"`. CLI11's auto help didn't print the JUCE version; add it to the help footer. - Windows --run-tests exit 127: runUnitTests() called the [[noreturn]] juce::ConsoleApplication::fail(), which throws; outside a ConsoleApplication command handler that throw was uncaught -> std::terminate. Set the application return value directly instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Linux build: include <CLI/CLI.hpp> before the JUCE headers. JUCE pulls in X11 on Linux (JUCE_GUI_BASICS_INCLUDE_XHEADERS), whose `#define Success 0` clashed with CLI11's CLI::ExitCodes::Success enumerator. macOS/Windows don't use X11, so only Linux was affected. - runUnitTests now prints each failed test (name + messages) to stdout. juce::UnitTestRunner logs via juce::Logger, which on a GUI app (Windows) doesn't reach the console, so CI couldn't show which tests failed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
"Command line parser" asserted opts.dataFile/outputDir.getFullPathName() against Unix-style literals; on Windows juce::File normalises those to the current drive (e.g. "D:\path\to\file"), failing the comparison. Assert the raw parsed settings strings instead, which are identical on all platforms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reworks the layering so a --config file (deliberate, per-invocation) ranks above ambient environment variables, and makes --config repeatable. - Environment layer: env-var names are now derived from the registered CLI11 options (--strictness-level -> STRICTNESS_LEVEL) instead of CLI11's ->envname(), so there is no hand-maintained env table and env support is automatic for every option. A synthetic "--name=value" argv is built from the environment and parsed by CLI11, reusing all its coercion (comma lists, enum transformer, hex-seed callback). Flags accept --flag=1/0. - --config: repeatable; each JSON file is merge_patch-ed in command-line order (later files win per key). Applied between the env and CLI layers, so it beats env and loses to explicit CLI options. (Inline JSON dropped for now to avoid command-line re-tokenisation hazards; file paths only.) - The env layer reads through an injectable EnvProvider again, so the unit tests no longer mutate the real process environment (also removes the Windows _putenv_s fragility). - Tests updated for the new precedence + a repeatable-config test. - Help footer, docs and CLAUDE.md updated. Verified end-to-end: env(6) -> 6; env(6)+config(2) -> 2; +CLI(9) -> 9. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 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>
- --validate-in-process is not a CLI option (launchInProcess is GUI/internal only); the new strict parser would now error on it. Removed from the options list and corrected the process-model notes (GUI uses child processes; the CLI --validate runs in-process with signal-handler crash reporting). - Note the staged 2.0.0 CHANGELIST entry vs the un-bumped VERSION. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Source/tests/EditorTests.cpp and Source/tests/ExtremeTests.cpp were never added to SourceFiles, so their self-registering PluginTest instances were never compiled in and the tests never ran. Add them to the build. Re-enables: "Editor stress" (L6), "Editor DPI Awareness" (L3, Windows only), "Allocations during process" (L9), "Process called with a larger than prepared block size". Verified they compile and pass a real strictness-10 validation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Qualify juce::ScopedDPIAwarenessDisabler. The EditorDPITest block is guarded by #if JUCE_WINDOWS && JUCE_WIN_PER_MONITOR_DPI_AWARE, so this latent bug only surfaced now that EditorTests.cpp is compiled (and only on Windows). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Superseded by #175, which targets the new |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the hand-rolled
juce::ArgumentListCLI parser with one plain settings struct (PluginvalSettings) that CLI11 binds to directly, and adds a new--config <file.json>option. CLI + environment-variable behaviour is preserved; the flat flags are unchanged (subcommands are a planned follow-up).Design
CLI11 binds each option straight to a
PluginvalSettingsmember, with the env var on the same line:--config <file.json>seeds the struct first; CLI11 only overwrites members whose flag/env was provided, so precedence is defaults < config < env < CLI (CLI wins) with no manual layering.nlohmann/jsonprovides--configload + the child-process handoff (an authoritative base64 JSON arg,--config-base64, avoiding command-line quoting hazards).PluginvalSettings::toPluginTestOptions()converts to the existingPluginTests::Optionsat the JUCE boundary (unchanged downstream).Adding an option is 3 edits: a struct member, a
nlohmannmacro entry, and oneadd_option(...)line. No parallel struct, no manual CLI→JSON or ENV→JSON code.Key changes
PluginvalSettings.h,SettingsParser.{h,cpp},SettingsSerializer.{h,cpp}.CommandLine.{h,cpp}reduced to a thin adapter (~395 lines of old parser removed);--run-tests/--strictness-helpdispatched by token scan,--help/--versionhandled by CLI11 (auto usage + a footer with env-var/commands notes).--config <file.json>; precedence covered by tests.CommandLineTests.cpprewritten against the new pipeline (env tests usesetenv/unsetenvsince CLI11 reads the real environment).Build / deps
cxx_std_20and macOS10.11. (CLI11 needs neither C++23 nor a newer macOS.)Testing
--run-testspasses (all 21 CommandLineTests + existing suites).--validateSUCCEEDs on multiple VST3s at strictness 5 and 10 (full build with embedded VST3 validator, macOS 10.11 target).--config+ CLI-override precedence verified by tests.Follow-up (not in this PR)
Restructure the mode flags into subcommands (
pluginval validatedefault /run-tests/strictness-help), keeping the old flat flags as deprecated aliases for one release. The settings struct + pipeline are designed to be reused unchanged.