Remove os.Exit callsites below the entrypoint: phase 2 (#2202)

* Remove os.Exit callsites below the entrypoint: phase 2 (plans/exit.md)

Phase 2 of plans/exit.md: the FlagParser signature change.

- FlagParser gains an error return; all 259 inline parser closures in
  option_parse.go and NoOpParse1 updated (mechanical rewrite, compiler-checked).
- FlagTable.Parse returns (bool, error); its nine callers (climain passes one
  and two, .mlrrc line handling, verb-local flag parsing in tee/join/put/
  filter/split, and the repl/script terminals) propagate the error instead of
  letting parsers kill the process.
- cli.CheckArgCount returns its missing-argument message as an error rather
  than printing and exiting; new cli.FlagErrorf (counterpart of VerbErrorf)
  carries user-facing main-flag messages.
- The 14 print-and-exit sites inside parser closures become returned errors;
  --list-color-codes/--list-color-names return ExitRequest{0} after printing.
- The repl and script terminals translate ExitRequest into their int return
  codes, so 'mlr repl --list-color-codes' still exits 0.
- pkg/cli is now os.Exit-free; stale comments about exiting parsers updated
  (the completion introspection accessors remain, since parsers mutate the
  options struct).

Stderr messages and exit codes are byte-identical, including the two-line
missing-argument and --seed messages and the trailing-period forms pinned by
test/cases/cli-mfrom/0003.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Merge main (recutils #2201); convert its three new parser closures

Same mechanical rewrite as the rest of phase 2: error return signature plus
final 'return nil' on --irecutils/--orecutils/--recutils.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
John Kerl 2026-07-15 14:09:57 -04:00 committed by GitHub
parent b7804e0791
commit be19a21280
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 770 additions and 385 deletions

View file

@ -1,6 +1,9 @@
package cli
import "errors"
import (
"errors"
"fmt"
)
// ErrHelpRequested is returned by verb ParseCLIFunc when -h or --help is used.
// The caller (CLI layer) should exit with code 0 after the verb has printed its
@ -11,3 +14,11 @@ var ErrHelpRequested = errors.New("help requested")
// verb has already printed its usage to stderr. The caller should exit 1
// without printing the error again.
var ErrUsagePrinted = errors.New("usage printed")
// FlagErrorf is for user-facing main-flag errors (the counterpart of
// VerbErrorf for verb flags). The message is complete prose, ready for the
// entrypoint layer to print as-is: it should carry the "mlr: " prefix and may
// span multiple lines.
func FlagErrorf(format string, args ...interface{}) error {
return fmt.Errorf(format, args...)
}

View file

@ -55,12 +55,15 @@ import (
// the latter case, and mutating the options struct.
// - Successful handling of the flag is indicated by this function making a
// non-zero increment of *pargi.
// - Invalid or missing flag arguments are reported by returning a non-nil
// error, which the caller propagates up to the entrypoint layer (see
// plans/exit.md); parsers must not call os.Exit.
type FlagParser func(
args []string,
argc int,
pargi *int,
options *TOptions,
)
) error
// FlagTable holds all the flags for Miller, organized into sections.
type FlagTable struct {
@ -132,13 +135,15 @@ func (ft *FlagTable) Sort() {
// Parse is for parsing a flag on the command line. Given say `--foo`, if a
// Flag object is found which owns the flag, and if its parser accepts it (e.g.
// `bar` is present and spelt correctly if the flag-parser expects `--foo bar`)
// then the return value is true, else false.
// then the boolean return value is true, else false. A non-nil error means the
// flag was recognized but its argument was missing or invalid; the caller
// should propagate it rather than continuing to parse.
func (ft *FlagTable) Parse(
args []string,
argc int,
pargi *int,
options *TOptions,
) bool {
) (bool, error) {
for _, section := range ft.sections {
for _, flag := range section.flags {
if flag.Owns(args[*pargi]) {
@ -146,13 +151,15 @@ func (ft *FlagTable) Parse(
// arguments follow the flag. E.g. `--ifs pipe` will advance
// *pargi by 2; `-I` will advance it by 1.
oargi := *pargi
flag.parser(args, argc, pargi, options)
if err := flag.parser(args, argc, pargi, options); err != nil {
return false, err
}
nargi := *pargi
return nargi > oargi
return nargi > oargi, nil
}
}
}
return false
return false, nil
}
// ShowHelp prints all-in-one on-line help, nominally for `mlr help flags`.
@ -517,14 +524,15 @@ func (flag *Flag) NilCheck() {
// NoOpParse1 is a helper function for flags which take no argument and are
// backward-compatibility no-ops.
func NoOpParse1(args []string, argc int, pargi *int, options *TOptions) {
func NoOpParse1(args []string, argc int, pargi *int, options *TOptions) error {
*pargi += 1
return nil
}
// Introspection accessors for shell-completion support
// (pkg/terminals/completion). These expose flag names and arity without
// invoking the parser functions, which is important since some flag parsers
// os.Exit on missing arguments.
// invoking the parser functions, which is important since the parser
// functions mutate the options struct (and can consume arguments).
// TakesArg reports whether the flag consumes a following argument value, e.g.
// `--ifs {string}` takes an argument whereas `--repifs` does not.
@ -559,8 +567,8 @@ func (ft *FlagTable) GetFlagNames() []string {
// FlagTakesArg looks up a main flag by any of its spellings. The first return
// value reports whether the flag is a recognized main flag; the second reports
// whether it consumes a following argument value. This drives the
// shell-completion command-line walk without calling the (possibly
// os.Exit-ing) parser functions.
// shell-completion command-line walk without calling the (options-mutating)
// parser functions.
func (ft *FlagTable) FlagTakesArg(name string) (found bool, takesArg bool) {
for _, section := range ft.sections {
for _, flag := range section.flags {

View file

@ -1,18 +1,16 @@
package cli
import (
"fmt"
"os"
)
// CheckArgCount is for flags with values, e.g. ["-n" "10"], while we're
// looking at the "-n": this let us see if the "10" slot exists.
func CheckArgCount(args []string, argi int, argc int, n int) {
// looking at the "-n": this let us see if the "10" slot exists. A non-nil
// return is a (multi-line) user-facing error, ready for printing as-is by the
// entrypoint layer.
func CheckArgCount(args []string, argi int, argc int, n int) error {
if (argc - argi) < n {
fmt.Fprintf(os.Stderr, "%s: option \"%s\" missing argument(s).\n", "mlr", args[argi])
fmt.Fprintf(os.Stderr, "Please run \"%s --help\" for detailed usage information.\n", "mlr")
os.Exit(1)
return FlagErrorf(
"%s: option \"%s\" missing argument(s).\nPlease run \"%s --help\" for detailed usage information.",
"mlr", args[argi], "mlr")
}
return nil
}
// SeparatorFromArg is for letting people do things like `--ifs pipe`

File diff suppressed because it is too large Load diff

View file

@ -163,7 +163,11 @@ func tryLoadMlrrc(
continue
}
if !handleMlrrcLine(options, stripped) {
handled, err := handleMlrrcLine(options, stripped)
if err != nil {
return true, err
}
if !handled {
return true, fmt.Errorf(
"parse error at file \"%s\" line %d: %s", path, lineno, line,
)
@ -198,11 +202,13 @@ func parseMlrrcSectionHeader(line string) (string, bool) {
}
// handleMlrrcLine is a helper function for tryLoadMlrrc, handling a single
// (comment-stripped, whitespace-trimmed, non-empty) settings line.
// (comment-stripped, whitespace-trimmed, non-empty) settings line. The boolean
// return says whether the line was recognized at all; a non-nil error is a
// flag-argument error to be propagated as-is.
func handleMlrrcLine(
options *cli.TOptions,
line string,
) bool {
) (bool, error) {
// Prepend initial "--" if it's not already there
if !strings.HasPrefix(line, "-") {
line = "--" + line
@ -213,21 +219,18 @@ func handleMlrrcLine(
argi := 0
argc := len(args)
if args[0] == "--prepipe" || args[0] == "--prepipex" {
switch args[0] {
case "--prepipe", "--prepipex":
// Don't allow code execution via .mlrrc
return false
} else if args[0] == "--load" || args[0] == "--mload" {
return false, nil
case "--load", "--mload":
// Don't allow code execution via .mlrrc
return false
} else if args[0] == "--profile" || args[0] == "-P" {
return false, nil
case "--profile", "-P":
// Profiles are selected on the mlr command line, not from within a
// .mlrrc file
return false
} else if cli.FLAG_TABLE.Parse(args, argc, &argi, options) {
// handled
} else {
return false
return false, nil
}
return true
return cli.FLAG_TABLE.Parse(args, argc, &argi, options)
}

View file

@ -173,7 +173,11 @@ func parseCommandLinePassOne(
argi += 1
flagSequences = append(flagSequences, args[oargi:argi])
} else if cli.FLAG_TABLE.Parse(args, argc, &argi, options) {
} else if handled, flagErr := cli.FLAG_TABLE.Parse(args, argc, &argi, options); flagErr != nil {
parseErr = flagErr
return
} else if handled {
flagSequences = append(flagSequences, args[oargi:argi])
} else if args[argi] == "--" {
@ -357,7 +361,10 @@ func parseCommandLinePassTwo(
lib.InternalCodingErrorIf(argc == 0)
// Parse the main-flag into the options struct.
rc := cli.FLAG_TABLE.Parse(args, argc, &argi, options)
rc, flagErr := cli.FLAG_TABLE.Parse(args, argc, &argi, options)
if flagErr != nil {
return nil, nil, flagErr
}
// Should have been parsed OK in pass one.
lib.InternalCodingErrorIf(!rc)

View file

@ -20,12 +20,14 @@
package repl
import (
"errors"
"fmt"
"os"
"path"
"strings"
"github.com/johnkerl/miller/v6/pkg/cli"
"github.com/johnkerl/miller/v6/pkg/lib"
)
func replUsage(verbName string, o *os.File, exitCode int) {
@ -140,7 +142,15 @@ func ReplMain(args []string) int {
argi += 1
}
} else if cli.FLAG_TABLE.Parse(args, argc, &argi, options) {
} else if handled, flagErr := cli.FLAG_TABLE.Parse(args, argc, &argi, options); flagErr != nil {
var exitRequest *lib.ExitRequest
if errors.As(flagErr, &exitRequest) {
return exitRequest.Code
}
fmt.Fprintf(os.Stderr, "%v\n", flagErr)
return 1
} else if handled {
} else {
replUsage(replName, os.Stderr, 1)

View file

@ -10,6 +10,7 @@
package script
import (
"errors"
"fmt"
"os"
"path"
@ -112,7 +113,14 @@ func ScriptMain(args []string) int {
if argi < argc && args[argi] == "--" {
argi++
}
} else if cli.FLAG_TABLE.Parse(args, argc, &argi, options) {
} else if handled, flagErr := cli.FLAG_TABLE.Parse(args, argc, &argi, options); flagErr != nil {
var exitRequest *lib.ExitRequest
if errors.As(flagErr, &exitRequest) {
return exitRequest.Code
}
fmt.Fprintf(os.Stderr, "%v\n", flagErr)
return 1
} else if handled {
} else {
scriptUsage(scriptName, os.Stderr, 1)
}

View file

@ -233,7 +233,11 @@ func transformerJoinParseCLI(
// loop (so individual if-statements don't need to). However,
// cli.Parse expects it unadvanced.
largi := argi - 1
if cli.FLAG_TABLE.Parse(args, argc, &largi, &opts.joinFlagOptions) {
handled, err := cli.FLAG_TABLE.Parse(args, argc, &largi, &opts.joinFlagOptions)
if err != nil {
return nil, err
}
if handled {
// This lets mlr main and mlr join have different input formats.
// Nothing else to handle here.
argi = largi

View file

@ -331,7 +331,11 @@ func transformerPutOrFilterParseCLI(
// loop (so individual if-statements don't need to). However,
// ParseWriterOptions expects it unadvanced.
largi := argi - 1
if cli.FLAG_TABLE.Parse(args, argc, &largi, options) {
handled, err := cli.FLAG_TABLE.Parse(args, argc, &largi, options)
if err != nil {
return nil, err
}
if handled {
// This lets mlr main and mlr put have different output formats.
// Nothing else to handle here.
argi = largi

View file

@ -185,7 +185,11 @@ func transformerSplitParseCLI(
// loop (so individual if-statements don't need to). However,
// ParseWriterOptions expects it unadvanced.
largi := argi - 1
if cli.FLAG_TABLE.Parse(args, argc, &largi, localOptions) {
handled, err := cli.FLAG_TABLE.Parse(args, argc, &largi, localOptions)
if err != nil {
return nil, err
}
if handled {
// This lets mlr main and mlr split have different output formats.
// Nothing else to handle here.
argi = largi

View file

@ -90,7 +90,11 @@ func transformerTeeParseCLI(
// loop (so individual if-statements don't need to). However,
// ParseWriterOptions expects it unadvanced.
largi := argi - 1
if cli.FLAG_TABLE.Parse(args, argc, &largi, localOptions) {
handled, err := cli.FLAG_TABLE.Parse(args, argc, &largi, localOptions)
if err != nil {
return nil, err
}
if handled {
// This lets mlr main and mlr tee have different output formats.
// Nothing else to handle here.
argi = largi