From 40ac1b309e55c1f9780707d8ac3ccd0313520e49 Mon Sep 17 00:00:00 2001 From: John Kerl Date: Wed, 15 Jul 2026 11:34:01 -0400 Subject: [PATCH] Remove `os.Exit` callsites below the entrypoint: phase 1 (#2198) * Rebuild docs: help-catalog index count drifted on main (666 -> 667) Pre-existing drift from a recent catalog addition; surfaced by make dev. Co-Authored-By: Claude Fable 5 * Remove os.Exit callsites below the entrypoint: phase 1 (plans/exit.md) Phase 1 of plans/exit.md: the mechanical swaps, plus the single-exit-point scaffolding. - New sentinel lib.ExitRequest{Code} (io.EOF-style control flow): returned by code paths that have already printed what they wanted (--version, -h, 'put --explain' on a valid expression, 'put -x') instead of exiting mid-stack. - pkg/entrypoint: new exitOnError() maps errors to exit codes in one place: ErrHelpRequested -> 0, ErrUsagePrinted -> 1, ExitRequest -> its code, anything else printed (JSON if --errors-json) -> 1. - pkg/climain: version/help/usage/validation exits become returned errors; the ErrHelpRequested/ErrUsagePrinted -> exit translation moves up to the entrypoint; loadMlrrcOrDie becomes error-returning loadMlrrcFiles. - Transformer ParseCLI/constructor exits -> returned errors (cut, having_fields, nest, reorder, merge_fields, reshape, rename, split, tee, subs, put_or_filter). merge_fields' bad-regex message formerly mis-reported itself as coming from the cut verb; tee's unrecognized-option exit was formerly silent; split/tee constructor failures formerly exited without printing the constructor's error. - YAML/JSON record-writer marshal errors -> returned through Write, matching the CSV writer's error path. - lib.WriteTempFileOrDie -> WriteTempFile (string, error); its sole caller (regtest diff helper) degrades gracefully. Stderr messages and exit codes are byte-identical for all regression-covered paths (4779 cases pass); runtime Transform-path exits are phase 3. Co-Authored-By: Claude Fable 5 * Add lib.NewExitZeroRequest() constructor for exit-0 sentinel returns Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- docs/src/ai-support.md | 2 +- pkg/climain/mlrcli_mlrrc.go | 20 ++------ pkg/climain/mlrcli_mlrrc_test.go | 32 ++++++------ pkg/climain/mlrcli_parse.go | 68 +++++++++++++++----------- pkg/entrypoint/entrypoint.go | 34 ++++++++++--- pkg/lib/exit.go | 24 +++++++++ pkg/lib/util.go | 13 ++--- pkg/output/record_writer_json_jsonl.go | 22 ++++----- pkg/output/record_writer_yaml.go | 28 +++++------ pkg/terminals/regtest/invoker.go | 13 ++++- pkg/transformers/cut.go | 7 +-- pkg/transformers/having_fields.go | 10 +--- pkg/transformers/merge_fields.go | 8 +-- pkg/transformers/nest.go | 14 +----- pkg/transformers/put_or_filter.go | 12 ++--- pkg/transformers/rename.go | 5 +- pkg/transformers/reorder.go | 7 +-- pkg/transformers/reshape.go | 7 +-- pkg/transformers/split.go | 3 +- pkg/transformers/subs.go | 8 +-- pkg/transformers/tee.go | 5 +- 21 files changed, 171 insertions(+), 171 deletions(-) create mode 100644 pkg/lib/exit.go diff --git a/docs/src/ai-support.md b/docs/src/ai-support.md index b7d4fcf8c..24f996707 100644 --- a/docs/src/ai-support.md +++ b/docs/src/ai-support.md @@ -64,7 +64,7 @@ one-line summary (here trimmed, and then counted, using Miller itself): 
 [
 {
-  "count": 666
+  "count": 667
 }
 ]
diff --git a/pkg/climain/mlrcli_mlrrc.go b/pkg/climain/mlrcli_mlrrc.go index 8d56eacd9..de09bcf61 100644 --- a/pkg/climain/mlrcli_mlrrc.go +++ b/pkg/climain/mlrcli_mlrrc.go @@ -11,19 +11,7 @@ import ( "github.com/johnkerl/miller/v6/pkg/cli" ) -// loadMlrrcOrDie is a fatal-error wrapper around loadMlrrc. -func loadMlrrcOrDie( - options *cli.TOptions, - profileName string, -) { - err := loadMlrrc(options, profileName) - if err != nil { - fmt.Fprintf(os.Stderr, "mlr: %v\n", err) - os.Exit(1) - } -} - -// loadMlrrc rule: If $MLRRC is set, use it and only it. Otherwise try first +// loadMlrrcFiles rule: If $MLRRC is set, use it and only it. Otherwise try first // $HOME/.mlrrc and then ./.mlrrc but let them stack: e.g. $HOME/.mlrrc is // lots of settings and maybe in one subdir you want to override just a // setting or two. @@ -34,7 +22,7 @@ func loadMlrrcOrDie( // skipped. Non-empty means global lines are applied first, then the lines in // any [profileName] section. It's a fatal error if a profile was requested // but no matching section exists in any .mlrrc file processed. -func loadMlrrc( +func loadMlrrcFiles( options *cli.TOptions, profileName string, ) error { @@ -85,7 +73,7 @@ func loadMlrrc( return checkMlrrcProfileWasFound(profileName, foundProfile, loadedPaths) } -// checkMlrrcProfileWasFound is a helper function for loadMlrrc: if a profile +// checkMlrrcProfileWasFound is a helper function for loadMlrrcFiles: if a profile // was requested via --profile {name} / -P {name}, there must be a matching // [name] section in at least one processed .mlrrc file. func checkMlrrcProfileWasFound( @@ -108,7 +96,7 @@ func checkMlrrcProfileWasFound( ) } -// tryLoadMlrrc is a helper function for loadMlrrc. The first return value is +// tryLoadMlrrc is a helper function for loadMlrrcFiles. The first return value is // whether the file could be opened at all: an unopenable file is not an error // (that's the normal case when no .mlrrc file exists). The second is any // parse error within an opened file. diff --git a/pkg/climain/mlrcli_mlrrc_test.go b/pkg/climain/mlrcli_mlrrc_test.go index 2b2efd8eb..c0f1b726d 100644 --- a/pkg/climain/mlrcli_mlrrc_test.go +++ b/pkg/climain/mlrcli_mlrrc_test.go @@ -39,7 +39,7 @@ func TestMlrrcGlobalOnlyBackCompat(t *testing.T) { // .mlrrc format. writeTempMlrrc(t, "icsv\nojson\n") options := cli.DefaultOptions() - if err := loadMlrrc(options, ""); err != nil { + if err := loadMlrrcFiles(options, ""); err != nil { t.Fatalf("unexpected error: %v", err) } if options.ReaderOptions.InputFileFormat != "csv" { @@ -53,7 +53,7 @@ func TestMlrrcGlobalOnlyBackCompat(t *testing.T) { func TestMlrrcSectionsIgnoredWithoutProfile(t *testing.T) { writeTempMlrrc(t, testMlrrcContents) options := cli.DefaultOptions() - if err := loadMlrrc(options, ""); err != nil { + if err := loadMlrrcFiles(options, ""); err != nil { t.Fatalf("unexpected error: %v", err) } if options.ReaderOptions.InputFileFormat != "csv" { @@ -68,7 +68,7 @@ func TestMlrrcSectionsIgnoredWithoutProfile(t *testing.T) { func TestMlrrcProfileSelection(t *testing.T) { writeTempMlrrc(t, testMlrrcContents) options := cli.DefaultOptions() - if err := loadMlrrc(options, "j"); err != nil { + if err := loadMlrrcFiles(options, "j"); err != nil { t.Fatalf("unexpected error: %v", err) } // Global setting applies first ... @@ -84,7 +84,7 @@ func TestMlrrcProfileSelection(t *testing.T) { func TestMlrrcRepeatedSectionsAccumulate(t *testing.T) { writeTempMlrrc(t, testMlrrcContents) options := cli.DefaultOptions() - if err := loadMlrrc(options, "j"); err != nil { + if err := loadMlrrcFiles(options, "j"); err != nil { t.Fatalf("unexpected error: %v", err) } // no-auto-flatten is in the second [j] block. @@ -96,7 +96,7 @@ func TestMlrrcRepeatedSectionsAccumulate(t *testing.T) { func TestMlrrcOtherProfileNotApplied(t *testing.T) { writeTempMlrrc(t, testMlrrcContents) options := cli.DefaultOptions() - if err := loadMlrrc(options, "p"); err != nil { + if err := loadMlrrcFiles(options, "p"); err != nil { t.Fatalf("unexpected error: %v", err) } if options.WriterOptions.OutputFileFormat != "pprint" { @@ -107,7 +107,7 @@ func TestMlrrcOtherProfileNotApplied(t *testing.T) { func TestMlrrcMissingProfileIsError(t *testing.T) { writeTempMlrrc(t, testMlrrcContents) options := cli.DefaultOptions() - err := loadMlrrc(options, "nonesuch") + err := loadMlrrcFiles(options, "nonesuch") if err == nil { t.Fatal("expected error for missing profile, got nil") } @@ -119,7 +119,7 @@ func TestMlrrcMissingProfileIsError(t *testing.T) { func TestMlrrcProfileWithMlrrcNoneIsError(t *testing.T) { t.Setenv("MLRRC", "__none__") options := cli.DefaultOptions() - err := loadMlrrc(options, "j") + err := loadMlrrcFiles(options, "j") if err == nil { t.Fatal("expected error for profile with MLRRC=__none__, got nil") } @@ -132,7 +132,7 @@ func TestMlrrcProfileWithNoMlrrcFileIsError(t *testing.T) { t.Setenv("MLRRC", filepath.Join(t.TempDir(), "nonexistent")) t.Setenv("HOME", t.TempDir()) options := cli.DefaultOptions() - err := loadMlrrc(options, "j") + err := loadMlrrcFiles(options, "j") if err == nil { t.Fatal("expected error for profile with no .mlrrc file, got nil") } @@ -144,7 +144,7 @@ func TestMlrrcProfileWithNoMlrrcFileIsError(t *testing.T) { func TestMlrrcWhitespaceAndCommentsAroundSectionHeaders(t *testing.T) { writeTempMlrrc(t, "icsv\n\n [ j ] # a comment\nojson\n") options := cli.DefaultOptions() - if err := loadMlrrc(options, "j"); err != nil { + if err := loadMlrrcFiles(options, "j"); err != nil { t.Fatalf("unexpected error: %v", err) } if options.WriterOptions.OutputFileFormat != "json" { @@ -156,7 +156,7 @@ func TestMlrrcMalformedSectionHeadersAreErrors(t *testing.T) { for _, header := range []string{"[j", "[]", "[ ]", "[a[b]"} { writeTempMlrrc(t, header+"\n") options := cli.DefaultOptions() - err := loadMlrrc(options, "") + err := loadMlrrcFiles(options, "") if err == nil { t.Errorf("expected parse error for header %q, got nil", header) } else if !strings.Contains(err.Error(), "parse error") { @@ -170,12 +170,12 @@ func TestMlrrcUnusedProfileLinesNotValidated(t *testing.T) { // invocations of mlr. writeTempMlrrc(t, "icsv\n[j]\nthis-is-not-a-flag\n") options := cli.DefaultOptions() - if err := loadMlrrc(options, ""); err != nil { + if err := loadMlrrcFiles(options, ""); err != nil { t.Fatalf("unexpected error: %v", err) } // But it is a parse error when the section is selected. options = cli.DefaultOptions() - if err := loadMlrrc(options, "j"); err == nil { + if err := loadMlrrcFiles(options, "j"); err == nil { t.Fatal("expected parse error for selected profile with bad line, got nil") } } @@ -183,7 +183,7 @@ func TestMlrrcUnusedProfileLinesNotValidated(t *testing.T) { func TestMlrrcGlobalParseErrorsStillFatal(t *testing.T) { writeTempMlrrc(t, "this-is-not-a-flag\n") options := cli.DefaultOptions() - if err := loadMlrrc(options, ""); err == nil { + if err := loadMlrrcFiles(options, ""); err == nil { t.Fatal("expected parse error, got nil") } } @@ -192,7 +192,7 @@ func TestMlrrcPrepipeStillDisallowed(t *testing.T) { // Code-execution flags are disallowed in .mlrrc -- inside profiles too. writeTempMlrrc(t, "[j]\nprepipe zcat\n") options := cli.DefaultOptions() - if err := loadMlrrc(options, "j"); err == nil { + if err := loadMlrrcFiles(options, "j"); err == nil { t.Fatal("expected error for --prepipe within a profile, got nil") } } @@ -203,7 +203,7 @@ func TestMlrrcProfileFlagDisallowedWithinMlrrc(t *testing.T) { for _, line := range []string{"profile j", "--profile j", "-P j"} { writeTempMlrrc(t, line+"\n") options := cli.DefaultOptions() - err := loadMlrrc(options, "") + err := loadMlrrcFiles(options, "") if err == nil { t.Errorf("expected parse error for %q within .mlrrc, got nil", line) } else if !strings.Contains(err.Error(), "parse error") { @@ -213,7 +213,7 @@ func TestMlrrcProfileFlagDisallowedWithinMlrrc(t *testing.T) { // Inside a selected profile section, too. writeTempMlrrc(t, "[j]\nprofile p\n") options := cli.DefaultOptions() - if err := loadMlrrc(options, "j"); err == nil { + if err := loadMlrrcFiles(options, "j"); err == nil { t.Fatal("expected parse error for --profile within a profile section, got nil") } } diff --git a/pkg/climain/mlrcli_parse.go b/pkg/climain/mlrcli_parse.go index 19512b312..cb4f8537b 100644 --- a/pkg/climain/mlrcli_parse.go +++ b/pkg/climain/mlrcli_parse.go @@ -151,18 +151,23 @@ func parseCommandLinePassOne( if args[argi][0] == '-' { if args[argi] == registry.VersionFlag { - // Exiting flag: handle it immediately. + // Exiting flag: print immediately, then request exit 0 from the + // entrypoint. fmt.Printf("mlr %s\n", version.STRING) - os.Exit(0) + parseErr = lib.NewExitZeroRequest() + return } else if args[argi] == registry.BareVersionFlag { - // Exiting flag: handle it immediately. + // Exiting flag: print immediately, then request exit 0 from the + // entrypoint. fmt.Printf("%s\n", version.STRING) - os.Exit(0) + parseErr = lib.NewExitZeroRequest() + return } else if help.ParseTerminalUsage(args[argi]) { - // Exiting flag: handle it immediately. + // Exiting flag: the usage text has been printed. // Most help is in the 'mlr help' terminal but there are a few // shorthands like 'mlr -h' and 'mlr -F'. - os.Exit(0) + parseErr = lib.NewExitZeroRequest() + return } else if args[argi] == "--norc" { argi += 1 @@ -196,13 +201,15 @@ func parseCommandLinePassOne( // The first verb in the then-chain can *optionally* be preceded by // 'then'. The others one *must* be. if args[argi] == "then" || args[argi] == "+" { - cli.CheckArgCount(args, argi, argc, 1) oargi++ argi++ } if argi >= argc { - fmt.Fprintln(os.Stderr, "mlr: 'then' must have a verb after it.") - os.Exit(1) + parseErr = &CLIError{ + Kind: "generic", + Msg: "mlr: 'then' must have a verb after it.", + } + return } verb := args[argi] onFirst = false @@ -226,11 +233,11 @@ func parseCommandLinePassOne( false, // false for first pass of CLI-parse, true for second pass -- this is the first pass ) if err != nil { - if errors.Is(err, cli.ErrHelpRequested) { - os.Exit(0) - } - if errors.Is(err, cli.ErrUsagePrinted) { - os.Exit(1) + if errors.Is(err, cli.ErrHelpRequested) || errors.Is(err, cli.ErrUsagePrinted) { + // Sentinel errors: any output has already been printed; the + // entrypoint maps these to their exit codes. + parseErr = err + return } // Return a structured error so the caller can emit JSON or // prose depending on --errors-json. @@ -268,7 +275,8 @@ func parseCommandLinePassOne( // piped stdin) defaults to 'cat'. if argc == 1 { help.MainUsage(os.Stderr) - os.Exit(1) + parseErr = cli.ErrUsagePrinted + return } verbSequences = append(verbSequences, []string{"cat"}) } @@ -326,13 +334,17 @@ func parseCommandLinePassTwo( } } if loadMlrrc { - loadMlrrcOrDie(options, mlrrcProfileName) + if err = loadMlrrcFiles(options, mlrrcProfileName); err != nil { + return nil, nil, err + } } else if mlrrcProfileName != "" { - fmt.Fprintf(os.Stderr, - "mlr: --profile \"%s\" was specified along with --norc, which disables .mlrrc processing.\n", - mlrrcProfileName, - ) - os.Exit(1) + return nil, nil, &CLIError{ + Kind: "generic", + Msg: fmt.Sprintf( + "mlr: --profile \"%s\" was specified along with --norc, which disables .mlrrc processing.", + mlrrcProfileName, + ), + } } // Process the flag-sequences in order from pass one. We assume all the @@ -402,12 +414,8 @@ func parseCommandLinePassTwo( true, // false for first pass of CLI-parse, true for second pass -- this is pass two ) if err != nil { - if errors.Is(err, cli.ErrHelpRequested) { - os.Exit(0) - } - if errors.Is(err, cli.ErrUsagePrinted) { - os.Exit(1) - } + // Sentinel errors (ErrHelpRequested, ErrUsagePrinted) pass through + // here too; the entrypoint maps them to their exit codes. return nil, nil, err } // Unparsable verb-setups should have been found in pass one. @@ -455,8 +463,10 @@ func parseCommandLinePassTwo( } if options.DoInPlace && len(options.FileNames) == 0 { - fmt.Fprintf(os.Stderr, "%s: -I option (in-place operation) requires input files.\n", "mlr") - os.Exit(1) + return nil, nil, &CLIError{ + Kind: "generic", + Msg: "mlr: -I option (in-place operation) requires input files.", + } } if options.HaveRandSeed { diff --git a/pkg/entrypoint/entrypoint.go b/pkg/entrypoint/entrypoint.go index 9d88ed120..8e6f44031 100644 --- a/pkg/entrypoint/entrypoint.go +++ b/pkg/entrypoint/entrypoint.go @@ -4,6 +4,7 @@ package entrypoint import ( + "errors" "fmt" "os" "path" @@ -48,12 +49,7 @@ func Main() MainReturn { options, recordTransformers, err := climain.ParseCommandLine(os.Args) if err != nil { - if wantJSON { - climain.EmitStructuredError(err) - } else { - printError(err) - } - os.Exit(1) + exitOnError(err, wantJSON) } if !options.DoInPlace { @@ -62,8 +58,7 @@ func Main() MainReturn { err = processFilesInPlace(options) } if err != nil { - printError(err) - os.Exit(1) + exitOnError(err, false) } return MainReturn{ @@ -71,6 +66,29 @@ func Main() MainReturn { } } +// exitOnError terminates the process for a non-nil error from command-line +// parsing or stream processing. Sentinel errors carry their own exit codes +// and have already produced whatever output they wanted; any other error is +// printed (structured if --errors-json is active) and exits 1. This is +// intended to be the single os.Exit point below main; see plans/exit.md. +func exitOnError(err error, wantJSON bool) { + var exitRequest *lib.ExitRequest + switch { + case errors.Is(err, cli.ErrHelpRequested): + os.Exit(0) + case errors.Is(err, cli.ErrUsagePrinted): + os.Exit(1) + case errors.As(err, &exitRequest): + os.Exit(exitRequest.Code) + case wantJSON: + climain.EmitStructuredError(err) + os.Exit(1) + default: + printError(err) + os.Exit(1) + } +} + // printError prints err to stderr. Errors that already carry an "mlr" prefix // (e.g. "mlr stats1: ..." or "mlr: verb not found") are printed as-is to // avoid double-prefixing. diff --git a/pkg/lib/exit.go b/pkg/lib/exit.go new file mode 100644 index 000000000..9f1e3fad5 --- /dev/null +++ b/pkg/lib/exit.go @@ -0,0 +1,24 @@ +package lib + +import "fmt" + +// ExitRequest is a sentinel error requesting process termination with the +// given exit code. Code paths that have already produced all the output they +// want (e.g. --version, or 'mlr put --explain' on a valid expression) return +// an ExitRequest instead of calling os.Exit mid-stack; the entrypoint unwraps +// it with errors.As and exits with the requested code. This is control flow +// in the io.EOF style: an "error" that isn't a failure. See plans/exit.md. +type ExitRequest struct { + Code int +} + +func (e *ExitRequest) Error() string { + return fmt.Sprintf("exit status %d", e.Code) +} + +// NewExitZeroRequest is for the common case of requesting a normal, exit-0 +// termination: e.g. --version or -h, where the requested output has been +// printed and nothing further should run. +func NewExitZeroRequest() *ExitRequest { + return &ExitRequest{Code: 0} +} diff --git a/pkg/lib/util.go b/pkg/lib/util.go index 5811cc364..4024f0f28 100644 --- a/pkg/lib/util.go +++ b/pkg/lib/util.go @@ -158,27 +158,24 @@ func GetArrayKeysSorted(input map[string]string) []string { // WriteTempFile places the contents string into a temp file, which the caller // must remove. -func WriteTempFileOrDie(contents string) string { +func WriteTempFile(contents string) (string, error) { // Use "" as first argument to os.CreateTemp to use default directory. // Nominally "/tmp" or somesuch on all unix-like systems, but not for Windows. handle, err := os.CreateTemp("", "mlr-temp") if err != nil { - fmt.Printf("mlr: could not create temp file.\n") - os.Exit(1) + return "", fmt.Errorf("could not create temp file: %w", err) } _, err = handle.WriteString(contents) if err != nil { - fmt.Printf("mlr: could not populate temp file.\n") - os.Exit(1) + return "", fmt.Errorf("could not populate temp file: %w", err) } err = handle.Close() if err != nil { - fmt.Printf("mlr: could not finish write of temp file.\n") - os.Exit(1) + return "", fmt.Errorf("could not finish write of temp file: %w", err) } - return handle.Name() + return handle.Name(), nil } func StripEmpties(input []string) []string { diff --git a/pkg/output/record_writer_json_jsonl.go b/pkg/output/record_writer_json_jsonl.go index eb40c5382..79fa53f42 100644 --- a/pkg/output/record_writer_json_jsonl.go +++ b/pkg/output/record_writer_json_jsonl.go @@ -2,8 +2,6 @@ package output import ( "bufio" - "fmt" - "os" "github.com/johnkerl/miller/v6/pkg/cli" "github.com/johnkerl/miller/v6/pkg/mlrval" @@ -56,11 +54,9 @@ func (writer *RecordWriterJSON) Write( } if writer.writerOptions.WrapJSONOutputInOuterList { - writer.writeWithListWrap(outrec, context, bufferedOutputStream, outputIsStdout) - } else { - writer.writeWithoutListWrap(outrec, context, bufferedOutputStream, outputIsStdout) + return writer.writeWithListWrap(outrec, context, bufferedOutputStream, outputIsStdout) } - return nil + return writer.writeWithoutListWrap(outrec, context, bufferedOutputStream, outputIsStdout) } func (writer *RecordWriterJSON) writeWithListWrap( @@ -68,7 +64,7 @@ func (writer *RecordWriterJSON) writeWithListWrap( context *types.Context, bufferedOutputStream *bufio.Writer, outputIsStdout bool, -) { +) error { if outrec != nil { // Not end of record stream if !writer.wroteAnyRecords { bufferedOutputStream.WriteString("[\n") @@ -78,8 +74,7 @@ func (writer *RecordWriterJSON) writeWithListWrap( // can place it neatly with commas here (if the user requested them). s, err := outrec.FormatAsJSON(writer.jsonFormatting, outputIsStdout) if err != nil { - fmt.Fprintf(os.Stderr, "mlr: %v\n", err) - os.Exit(1) + return err } if writer.wroteAnyRecords { @@ -102,6 +97,7 @@ func (writer *RecordWriterJSON) writeWithListWrap( } } + return nil } func (writer *RecordWriterJSON) writeWithoutListWrap( @@ -109,20 +105,20 @@ func (writer *RecordWriterJSON) writeWithoutListWrap( _ *types.Context, bufferedOutputStream *bufio.Writer, outputIsStdout bool, -) { +) error { if outrec == nil { // End of record stream - return + return nil } // The Mlrmap FormatAsJSON doesn't include the final newline, so that we // can place it neatly with commas here (if the user requested them). s, err := outrec.FormatAsJSON(writer.jsonFormatting, outputIsStdout) if err != nil { - fmt.Fprintf(os.Stderr, "mlr: %v\n", err) - os.Exit(1) + return err } bufferedOutputStream.WriteString(s) bufferedOutputStream.WriteString("\n") + return nil } diff --git a/pkg/output/record_writer_yaml.go b/pkg/output/record_writer_yaml.go index d6f7d0602..32f592231 100644 --- a/pkg/output/record_writer_yaml.go +++ b/pkg/output/record_writer_yaml.go @@ -2,8 +2,6 @@ package output import ( "bufio" - "fmt" - "os" "gopkg.in/yaml.v3" @@ -33,25 +31,22 @@ func (writer *RecordWriterYAML) Write( outputIsStdout bool, ) error { if writer.writerOptions.WrapYAMLOutputInOuterList { - writer.writeWithListWrap(outrec, bufferedOutputStream) - } else { - writer.writeWithoutListWrap(outrec, bufferedOutputStream) + return writer.writeWithListWrap(outrec, bufferedOutputStream) } - return nil + return writer.writeWithoutListWrap(outrec, bufferedOutputStream) } func (writer *RecordWriterYAML) writeWithListWrap( outrec *mlrval.Mlrmap, bufferedOutputStream *bufio.Writer, -) { +) error { if outrec != nil { if writer.bufferedRecords == nil { writer.bufferedRecords = []*yaml.Node{} } native, err := mlrval.MlrmapToYAMLNative(outrec) if err != nil { - fmt.Fprintf(os.Stderr, "mlr: %v\n", err) - os.Exit(1) + return err } writer.bufferedRecords = append(writer.bufferedRecords, native) } else { @@ -60,34 +55,33 @@ func (writer *RecordWriterYAML) writeWithListWrap( seqNode.Content = writer.bufferedRecords out, err := yaml.Marshal(seqNode) if err != nil { - fmt.Fprintf(os.Stderr, "mlr: %v\n", err) - os.Exit(1) + return err } bufferedOutputStream.Write(out) writer.bufferedRecords = nil } + return nil } func (writer *RecordWriterYAML) writeWithoutListWrap( outrec *mlrval.Mlrmap, bufferedOutputStream *bufio.Writer, -) { +) error { if outrec == nil { - return + return nil } if writer.wroteAnyRecords { bufferedOutputStream.WriteString("---\n") } native, err := mlrval.MlrmapToYAMLNative(outrec) if err != nil { - fmt.Fprintf(os.Stderr, "mlr: %v\n", err) - os.Exit(1) + return err } out, err := yaml.Marshal(native) if err != nil { - fmt.Fprintf(os.Stderr, "mlr: %v\n", err) - os.Exit(1) + return err } bufferedOutputStream.Write(out) writer.wroteAnyRecords = true + return nil } diff --git a/pkg/terminals/regtest/invoker.go b/pkg/terminals/regtest/invoker.go index 9b930b7e3..71e498fad 100644 --- a/pkg/terminals/regtest/invoker.go +++ b/pkg/terminals/regtest/invoker.go @@ -2,6 +2,7 @@ package regtest import ( "bytes" + "fmt" "os" "os/exec" "strings" @@ -70,9 +71,17 @@ func RunDiffCommandOnStrings( ) ( diffOutput string, ) { - actualOutputFileName := lib.WriteTempFileOrDie(actualOutput) - expectedOutputFileName := lib.WriteTempFileOrDie(expectedOutput) + // The diff output is best-effort debugging help (see below); if we can't + // write the temp files for it, say so in its place rather than aborting. + actualOutputFileName, err := lib.WriteTempFile(actualOutput) + if err != nil { + return fmt.Sprintf("(diff unavailable: %v)", err) + } defer func() { _ = os.Remove(actualOutputFileName) }() + expectedOutputFileName, err := lib.WriteTempFile(expectedOutput) + if err != nil { + return fmt.Sprintf("(diff unavailable: %v)", err) + } defer func() { _ = os.Remove(expectedOutputFileName) }() // This is diff or fc diff --git a/pkg/transformers/cut.go b/pkg/transformers/cut.go index fad7f0b7f..69f170727 100644 --- a/pkg/transformers/cut.go +++ b/pkg/transformers/cut.go @@ -162,12 +162,7 @@ func NewTransformerCut( // Handles "a.*b"i Miller case-insensitive-regex specification regex, err := lib.CompileMillerRegex(regexString) if err != nil { - fmt.Fprintf( - os.Stderr, - "%s %s: cannot compile regex [%s]\n", - "mlr", verbNameCut, regexString, - ) - os.Exit(1) + return nil, cli.VerbErrorf(verbNameCut, "cannot compile regex [%s]", regexString) } tr.regexes[i] = regex } diff --git a/pkg/transformers/having_fields.go b/pkg/transformers/having_fields.go index c6bdc9656..aef3827f0 100644 --- a/pkg/transformers/having_fields.go +++ b/pkg/transformers/having_fields.go @@ -207,15 +207,7 @@ func NewTransformerHavingFields( // Strip off the leading " and trailing " or "i. regex, err := lib.CompileMillerRegex(regexString) if err != nil { - fmt.Fprintf( - os.Stderr, - "%s %s: cannot compile regex \"%s\"\n", - "mlr", - verbNameHavingFields, - regexString, - ) - os.Exit(1) - // return nil, err + return nil, cli.VerbErrorf(verbNameHavingFields, "cannot compile regex \"%s\"", regexString) } tr.regex = regex diff --git a/pkg/transformers/merge_fields.go b/pkg/transformers/merge_fields.go index 8e8d5928c..574ed28c7 100644 --- a/pkg/transformers/merge_fields.go +++ b/pkg/transformers/merge_fields.go @@ -282,12 +282,8 @@ func NewTransformerMergeFields( // Handles "a.*b"i Miller case-insensitive-regex specification regex, err := lib.CompileMillerRegex(regexString) if err != nil { - fmt.Fprintf( - os.Stderr, - "%s %s: cannot compile regex [%s]\n", - "mlr", verbNameCut, regexString, - ) - os.Exit(1) + // (This formerly mis-reported itself as coming from the cut verb.) + return nil, cli.VerbErrorf(verbNameMergeFields, "cannot compile regex [%s]", regexString) } tr.valueFieldNameRegexes[i] = regex } diff --git a/pkg/transformers/nest.go b/pkg/transformers/nest.go index e3fa14d26..914a288aa 100644 --- a/pkg/transformers/nest.go +++ b/pkg/transformers/nest.go @@ -313,12 +313,7 @@ func NewTransformerNest( if doRegexes { fieldRegex, err := lib.CompileMillerRegex(fieldName) if err != nil { - fmt.Fprintf( - os.Stderr, - "%s %s: cannot compile regex [%s]\n", - "mlr", verbNameNest, fieldName, - ) - os.Exit(1) + return nil, cli.VerbErrorf(verbNameNest, "cannot compile regex [%s]", fieldName) } tr.fieldRegex = fieldRegex // implode uses fieldRegex directly when doRegexes @@ -327,12 +322,7 @@ func NewTransformerNest( regexString := "^" + fieldName + "_[0-9]+$" regex, err := lib.CompileMillerRegex(regexString) if err != nil { - fmt.Fprintf( - os.Stderr, - "%s %s: cannot compile regex [%s]\n", - "mlr", verbNameNest, regexString, - ) - os.Exit(1) + return nil, cli.VerbErrorf(verbNameNest, "cannot compile regex [%s]", regexString) } tr.regex = regex } diff --git a/pkg/transformers/put_or_filter.go b/pkg/transformers/put_or_filter.go index 5351c3352..14f095ebb 100644 --- a/pkg/transformers/put_or_filter.go +++ b/pkg/transformers/put_or_filter.go @@ -459,7 +459,8 @@ func NewTransformerPut( // --explain is a validate/dry-run: report whether the DSL parsed and // type-checked, then exit without reading the input stream. A parse/build // error is returned so it flows through the normal error path (including - // --errors-json); a valid expression prints a confirmation and exits 0. + // --errors-json); a valid expression prints a confirmation and requests + // exit 0 via the ExitRequest sentinel. if doExplain { if err != nil { return nil, err @@ -469,11 +470,10 @@ func NewTransformerPut( verbName = "filter" } if warningsAreFatal && hadWarnings { - fmt.Fprintf(os.Stderr, "mlr %s: DSL expression has warnings treated as fatal.\n", verbName) - os.Exit(1) + return nil, cli.VerbErrorf(verbName, "DSL expression has warnings treated as fatal.") } fmt.Printf("mlr %s: DSL expression is valid.\n", verbName) - os.Exit(0) + return nil, lib.NewExitZeroRequest() } if warningsAreFatal && hadWarnings { @@ -481,11 +481,11 @@ func NewTransformerPut( "%s: Exiting due to warnings treated as fatal.\n", "mlr", ) - os.Exit(1) + return nil, &lib.ExitRequest{Code: 1} } if exitAfterParse { - os.Exit(0) + return nil, lib.NewExitZeroRequest() } if err != nil { diff --git a/pkg/transformers/rename.go b/pkg/transformers/rename.go index 428be04f1..d320461e0 100644 --- a/pkg/transformers/rename.go +++ b/pkg/transformers/rename.go @@ -164,7 +164,10 @@ func NewTransformerRename( tr.regexesAndReplacements = []*tRegexAndReplacement{} for pe := oldToNewNames.Head; pe != nil; pe = pe.Next { regexString := pe.Key - regex := lib.CompileMillerRegexOrDie(regexString) + regex, err := lib.CompileMillerRegex(regexString) + if err != nil { + return nil, err + } replacement := pe.Value _, replacementCaptureMatrix := lib.ReplacementHasCaptures(replacement) regexAndReplacement := tRegexAndReplacement{ diff --git a/pkg/transformers/reorder.go b/pkg/transformers/reorder.go index d5892c47c..b67030f54 100644 --- a/pkg/transformers/reorder.go +++ b/pkg/transformers/reorder.go @@ -196,12 +196,7 @@ func NewTransformerReorder( // Handles "a.*b"i Miller case-insensitive-regex specification regex, err := lib.CompileMillerRegex(regexString) if err != nil { - fmt.Fprintf( - os.Stderr, - "%s %s: cannot compile regex [%s]\n", - "mlr", verbNameReorder, regexString, - ) - os.Exit(1) + return nil, cli.VerbErrorf(verbNameReorder, "cannot compile regex [%s]", regexString) } tr.regexes[i] = regex } diff --git a/pkg/transformers/reshape.go b/pkg/transformers/reshape.go index 2231721c9..9c5ec488d 100644 --- a/pkg/transformers/reshape.go +++ b/pkg/transformers/reshape.go @@ -276,12 +276,7 @@ func NewTransformerReshape( for i, inputFieldRegexString := range inputFieldRegexStrings { regex, err := lib.CompileMillerRegex(inputFieldRegexString) if err != nil { - fmt.Fprintf( - os.Stderr, - "%s %s: cannot compile regex [%s]\n", - "mlr", verbNameReshape, inputFieldRegexString, - ) - os.Exit(1) + return nil, cli.VerbErrorf(verbNameReshape, "cannot compile regex [%s]", inputFieldRegexString) } tr.inputFieldRegexes[i] = regex } diff --git a/pkg/transformers/split.go b/pkg/transformers/split.go index b9bc711cc..00b257856 100644 --- a/pkg/transformers/split.go +++ b/pkg/transformers/split.go @@ -230,8 +230,7 @@ func transformerSplitParseCLI( &localOptions.WriterOptions, ) if err != nil { - // Error message already printed out - os.Exit(1) + return nil, err } return transformer, nil diff --git a/pkg/transformers/subs.go b/pkg/transformers/subs.go index 23f5a70b8..f43181336 100644 --- a/pkg/transformers/subs.go +++ b/pkg/transformers/subs.go @@ -177,7 +177,7 @@ func transformerSubsParseCLI( switch opt { case "-h", "--help": usageFunc(os.Stdout) - os.Exit(0) + return nil, cli.ErrHelpRequested case "-a": doAllFieldNames = true @@ -195,19 +195,19 @@ func transformerSubsParseCLI( doAllFieldNames = false default: usageFunc(os.Stderr) - os.Exit(1) + return nil, cli.ErrUsagePrinted } } if fieldNames == nil && !doAllFieldNames { usageFunc(os.Stderr) - os.Exit(1) + return nil, cli.ErrUsagePrinted } // Get the old and new text from the command line if (argc - argi) < 2 { usageFunc(os.Stderr) - os.Exit(1) + return nil, cli.ErrUsagePrinted } oldText = args[argi] newText = args[argi+1] diff --git a/pkg/transformers/tee.go b/pkg/transformers/tee.go index 7a043a01f..0ebd9681f 100644 --- a/pkg/transformers/tee.go +++ b/pkg/transformers/tee.go @@ -95,7 +95,7 @@ func transformerTeeParseCLI( // Nothing else to handle here. argi = largi } else { - os.Exit(1) + return nil, cli.VerbErrorf(verbNameTee, "option \"%s\" not recognized", opt) } } } @@ -123,8 +123,7 @@ func transformerTeeParseCLI( &localOptions.WriterOptions, ) if err != nil { - // Error message already printed out - os.Exit(1) + return nil, err } return transformer, nil