Skip to content

feat(ParseConfig): Add record + Parse(config) overload (additive)#307

Merged
bartelink merged 3 commits into
fsprojects:masterfrom
dimension-zero:pr/19-parser-config-record
Jun 4, 2026
Merged

feat(ParseConfig): Add record + Parse(config) overload (additive)#307
bartelink merged 3 commits into
fsprojects:masterfrom
dimension-zero:pr/19-parser-config-record

Conversation

@dimension-zero

Copy link
Copy Markdown
Contributor

feat(ParseConfig): Add record + Parse(config) overload (additive)

  • ArgumentParser.fs: Introduce public ParseConfig record carrying
    Inputs, ConfigurationReader, IgnoreMissing, IgnoreUnrecognized,
    RaiseOnUsage. Provide ParseConfig.Default reproducing the historical
    Parse(...) defaults.
  • New Parse(config: ParseConfig) overload delegates to the existing
    Parse(?inputs, ?configurationReader, ...) implementation. All current
    overloads remain untouched, so existing callers see no shape change.

Useful when hosts want to layer configuration programmatically (e.g.
merging container-provided defaults with user overrides) rather than
juggling many optional method arguments.


test(ParseConfig): Add coverage for ParseConfig record + Parse(config)

6 new tests:

  • ParseConfig.Default holds the historical defaults (None inputs, None
    reader, false IgnoreMissing/IgnoreUnrecognized, true RaiseOnUsage).
  • Parse(config with explicit inputs) parses those inputs.
  • Parse(config) matches Parse(?inputs, ...) for the same shape.
  • IgnoreMissing=true skips the mandatory check on empty inputs.
  • IgnoreUnrecognized=true gathers unknown args into UnrecognizedCliParams.
  • ConfigurationReader provided via config sources AppSettings entries.

A separate non-mandatory schema is used for the AppSettings test because
Argu's missing-mandatory check fires from CLI parsing even when AppSettings
provides a value (existing behaviour, not introduced by PR 19).

Net suite size on this branch: 112 -> 118.


Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an additive ParseConfig record and a new Parse(config: ParseConfig) overload on ArgumentParser<'Template> that simply forwards to the existing optional-argument Parse overload. Intended for callers that prefer to assemble parse parameters as data (e.g. layering host defaults over user overrides) rather than through many optional method arguments. Existing overloads are untouched, so this is non-breaking.

Changes:

  • Introduce public ParseConfig record (with Default matching historical Parse defaults) in the Argu namespace.
  • Add ArgumentParser<'Template>.Parse(config: ParseConfig) overload that delegates to the existing optional-argument Parse.
  • Add a new ParseConfigTests.fs covering defaults, explicit inputs, parity with the optional overload, IgnoreMissing, IgnoreUnrecognized, and ConfigurationReader.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Argu/ArgumentParser.fs Adds ParseConfig record + Default and a new Parse(config) overload forwarding to existing Parse.
tests/Argu.Tests/ParseConfigTests.fs New xUnit/Unquote tests for the ParseConfig record and Parse(config) overload.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/Argu.Tests/ParseConfigTests.fs Outdated
* ArgumentParser.fs: Introduce public ParseConfig record carrying
  Inputs, ConfigurationReader, IgnoreMissing, IgnoreUnrecognized,
  RaiseOnUsage. Provide ParseConfig.Default reproducing the historical
  Parse(...) defaults.
* New Parse(config: ParseConfig) overload delegates to the existing
  Parse(?inputs, ?configurationReader, ...) implementation. All current
  overloads remain untouched, so existing callers see no shape change.

Useful when hosts want to layer configuration programmatically (e.g.
merging container-provided defaults with user overrides) rather than
juggling many optional method arguments.
6 new tests:
* ParseConfig.Default holds the historical defaults (None inputs, None
  reader, false IgnoreMissing/IgnoreUnrecognized, true RaiseOnUsage).
* Parse(config with explicit inputs) parses those inputs.
* Parse(config) matches Parse(?inputs, ...) for the same shape.
* IgnoreMissing=true skips the mandatory check on empty inputs.
* IgnoreUnrecognized=true gathers unknown args into UnrecognizedCliParams.
* ConfigurationReader provided via config sources AppSettings entries.

A separate non-mandatory schema is used for the AppSettings test because
Argu's missing-mandatory check fires from CLI parsing even when AppSettings
provides a value (existing behaviour, not introduced by PR 19).

Net suite size on this branch: 112 -> 118.

# Conflicts:
#	tests/Argu.Tests/Argu.Tests.fsproj
@bartelink bartelink force-pushed the pr/19-parser-config-record branch 2 times, most recently from 4e33ca2 to 38f5d32 Compare June 4, 2026 14:35
@bartelink bartelink force-pushed the pr/19-parser-config-record branch from 38f5d32 to 99f1f25 Compare June 4, 2026 14:38
@bartelink bartelink merged commit 1c92258 into fsprojects:master Jun 4, 2026
4 checks passed
@bartelink

Copy link
Copy Markdown
Member

@dimension-zero See #317 (comment) #317 (comment)

I feel it'd be a net win to take ParseConfig out of the picture now (i.e. revert this PR) and focus instead on the extensions layering and rationalizing the top level ParseCommand vs ParseWith(argv, configReader) vs ParseWithAsync(argv, asyncConfigReader) split

All going well, that clarifies matters to the extent that this becomes entirely superfluous

@bartelink bartelink mentioned this pull request Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants