Skip to content

refactor(extensions)!: centralize argument extraction - #205

Merged
wackywendell merged 1 commit into
mainfrom
wendell/framework-managed-args-extractor
Aug 17, 2026
Merged

refactor(extensions)!: centralize argument extraction#205
wackywendell merged 1 commit into
mainfrom
wendell/framework-managed-args-extractor

Conversation

@wackywendell

@wackywendell wackywendell commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR simplifies the interface around ArgsExtractor, and pushes the assertions around using all arguments (check_exhausted()) out of user code and into the registry conversion.

fn from_args(args: &mut ArgsAccess<'_>) -> Result<Self, ExtensionError> {
    let path = args.expect_named::<&str>("path")?.to_owned();
    let batch_size = args.get_named::<i64>("batch_size")?.unwrap_or(1024);
    Ok(Self { path, batch_size })
}

Changes

  • Changes Explainable::from_args to receive &mut ArgsAccess<'_>.
  • Removes ExtensionArgs::extractor, public ArgsExtractor::check_exhausted, and the panic-based Drop assertion.
  • Rejects unhandled named and positional arguments after successful decoding.
  • Makes positional() mark the complete positional list as handled.
  • Keeps output_columns() read-only and outside exhaustion checking because output columns are relation metadata consumed independently by the structural parser.
  • Preserves Explainable::to_args(&ExtensionContext) and context-aware relation formatting from feat(extensions)!: Add context to to_args in Explainable trait #202.
  • Rewrites the API documentation around the user-visible decoding lifecycle rather than presenting ArgsExtractor as an optional convenience.

API impact

This is a breaking public API change. External Explainable implementations must:

  • change from_args(&ExtensionArgs) to from_args(&mut ArgsExtractor<'_>);
  • use the extractor passed by the registry;
  • remove explicit check_exhausted() calls.

There is intentionally no new public direct-conversion helper. The registry owns the checked decoding boundary.

Validation

  • cargo test --all-features
  • just check
  • Independent review found no actionable issues

@wackywendell
wackywendell force-pushed the wendell/framework-managed-args-extractor branch from c42c288 to 665b375 Compare August 10, 2026 21:36
@wackywendell
wackywendell force-pushed the wendell/framework-managed-args-extractor branch from 665b375 to e72c6d2 Compare August 12, 2026 14:39
@wackywendell
wackywendell force-pushed the wendell/framework-managed-args-extractor branch from e72c6d2 to 12306b6 Compare August 12, 2026 16:31
@wackywendell
wackywendell force-pushed the wendell/framework-managed-args-extractor branch from 12306b6 to 0fdd9b3 Compare August 12, 2026 20:13
Base automatically changed from wendell/extension-argument-handling to main August 13, 2026 15:07
@wackywendell
wackywendell force-pushed the wendell/framework-managed-args-extractor branch 2 times, most recently from 2dcd59b to 1259d88 Compare August 13, 2026 18:07
@wackywendell
wackywendell force-pushed the wendell/framework-managed-args-extractor branch from 1259d88 to a6d4042 Compare August 14, 2026 15:36
@wackywendell
wackywendell marked this pull request as ready for review August 14, 2026 15:43
@wackywendell
wackywendell requested a review from a team as a code owner August 14, 2026 15:43

@edouardmulliez edouardmulliez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good refacto!

@wackywendell
wackywendell merged commit f4eab15 into main Aug 17, 2026
4 checks passed
@wackywendell
wackywendell deleted the wendell/framework-managed-args-extractor branch August 17, 2026 16:39
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.

2 participants