Skip to content

Add FlaggedCommand interface so the framework owns flag parsing#27

Merged
thieman merged 5 commits intomainfrom
thieman/flags-interface
Mar 10, 2026
Merged

Add FlaggedCommand interface so the framework owns flag parsing#27
thieman merged 5 commits intomainfrom
thieman/flags-interface

Conversation

@thieman
Copy link
Copy Markdown
Collaborator

@thieman thieman commented Mar 10, 2026

Introduce FlaggedCommand/FlaggedHandlerFunc to builtins.go: the framework creates a *pflag.FlagSet per invocation, calls the command's MakeFlags factory to register flags and get a bound handler, then runs Parse and error reporting before dispatching with positional args only. Commands no longer contain boilerplate for fs.SetOutput, fs.Parse, or parse-error formatting.

Expose type FlagSet = pflag.FlagSet via the builtins package so command files can use *builtins.FlagSet in their factory signature without importing pflag directly (the builtins package is always-allowed in the import allowlist).

Add Registrable interface (Register method on both Command and FlaggedCommand) so register_builtins.go can hold both types in one slice.

Convert head to FlaggedCommand as the first consumer of the new interface.

thieman and others added 2 commits March 10, 2026 12:11
Introduce FlaggedCommand/FlaggedHandlerFunc to builtins.go: the framework
creates a *pflag.FlagSet per invocation, calls the command's MakeFlags factory
to register flags and get a bound handler, then runs Parse and error reporting
before dispatching with positional args only. Commands no longer contain
boilerplate for fs.SetOutput, fs.Parse, or parse-error formatting.

Expose type FlagSet = pflag.FlagSet via the builtins package so command files
can use *builtins.FlagSet in their factory signature without importing pflag
directly (the builtins package is always-allowed in the import allowlist).

Add Registrable interface (Register method on both Command and FlaggedCommand)
so register_builtins.go can hold both types in one slice.

Convert head to FlaggedCommand as the first consumer of the new interface.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the parallel Command/FlaggedCommand/HandlerFunc/BoundHandlerFunc/
FlaggedHandlerFunc/Registrable surface with one type each:

  Command  { Name string; MakeFlags func(*FlagSet) HandlerFunc }
  HandlerFunc  func(ctx, callCtx, args) Result

Every command declares its flags via MakeFlags. The framework creates a fresh
*FlagSet per invocation, calls MakeFlags to register flags and get the bound
handler, then parses raw args before calling the handler with positional args.

Commands with no flags (echo, cat, break, continue, true, false, exit) use the
NoFlags helper which wraps a plain HandlerFunc. The framework detects that no
flags were registered via fs.HasFlags() and skips parsing entirely, so
flag-shaped literals like "echo -n hello" are passed through unchanged.

register_builtins.go now iterates a single []builtins.Command.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thieman thieman marked this pull request as ready for review March 10, 2026 16:28
thieman and others added 3 commits March 10, 2026 12:33
pflag.ContinueOnError, pflag.NewFlagSet, and io.Discard moved into
builtins.go (the framework layer, exempt from the allowlist) as part of
the FlaggedCommand consolidation. No command implementation file references
these symbols anymore.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Merge from main brought in a reimplemented cat and a new wc command,
both using the old Run field and manually calling pflag.NewFlagSet /
io.Discard inside their run functions. This broke the build (unknown
field Run) and would have failed the import allowlist (pflag.NewFlagSet,
pflag.ContinueOnError, and io.Discard were removed from the allowlist in
this PR).

Convert both to registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc,
removing the pflag import from each file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thieman thieman merged commit ce5c312 into main Mar 10, 2026
8 checks passed
@thieman thieman deleted the thieman/flags-interface branch March 10, 2026 16:46
julesmcrt added a commit that referenced this pull request Mar 10, 2026
- Convert tail.go from Run to MakeFlags/HandlerFunc pattern, matching
  the framework change merged from origin/main (PR #27).
- Remove pflag import; framework now owns FlagSet creation and parsing.
- Drop pflag.ContinueOnError, pflag.NewFlagSet, and io.Discard from the
  import allowlist — no builtin implementation file uses them directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
AlexandreYang added a commit that referenced this pull request Mar 10, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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