mirror of
https://github.com/johnkerl/miller.git
synced 2026-07-17 16:38:54 +00:00
plans/exit.md: record final status; defer phase 5; convert stragglers (#2207)
Phase-5 wrap-up for plans/exit.md. The DSL-runtime cluster (pkg/dsl/cst: hofs.go, udf.go, evaluable.go) is deferred rather than converted: 58 regression cases pin the current loud-failure behavior (exact stderr plus exit 1), and the planned error-Mlrval mechanism would make HOF/UDF misuse fail silently as bare '(error)' with exit 0 -- a debugging-UX regression. The alternative (typed panic recovered at the Execute* boundaries) preserves behavior but introduces panic/recover to a codebase that has none. The decision belongs with the issue-440 strict-mode design, where the fatal-vs-data error taxonomy gets decided anyway; plans/exit.md now records the rationale, the phase-by-phase status (#2198, #2202, #2204, #2205), and the final intentional keep-list. Also converted here, since they're squarely in prior phases' patterns rather than the deferred expression-depth cluster: - RootNode.ProcessEndOfStream (a phase-3 leftover on the put/filter Transform path) returns an error instead of printing-and-exiting on end-of-stream close failures; first error returned, any others printed at the site. - The three CompileMillerRegexOrDie calls in option_parse.go (inside error-returning parser closures since phase 2) use CompileMillerRegex and return the error; 'mlr --ifs-regex (' output and exit code are unchanged. All 4779 regression cases pass; make lint 0 issues. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
2f3e54d1f6
commit
71190d3d99
4 changed files with 69 additions and 7 deletions
|
|
@ -59,7 +59,11 @@ func FinalizeReaderOptions(readerOptions *TReaderOptions) error {
|
|||
// and spaces, that should now be the default for NIDX. But *only* for NIDX format,
|
||||
// and if IFS wasn't specified.
|
||||
if readerOptions.InputFileFormat == "nidx" && !readerOptions.ifsWasSpecified {
|
||||
readerOptions.IFSRegex = lib.CompileMillerRegexOrDie(WHITESPACE_REGEX)
|
||||
regex, err := lib.CompileMillerRegex(WHITESPACE_REGEX)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
readerOptions.IFSRegex = regex
|
||||
} else {
|
||||
if allowRepeatIFS, ok := defaultAllowRepeatIFSes[readerOptions.InputFileFormat]; ok {
|
||||
readerOptions.AllowRepeatIFS = allowRepeatIFS
|
||||
|
|
@ -285,7 +289,11 @@ var SeparatorFlagSection = FlagSection{
|
|||
// LF vs CR/LF line endings is handled within Go libraries so
|
||||
// we needn't do anything ourselves.
|
||||
if args[*pargi+1] != "auto" {
|
||||
options.ReaderOptions.IFSRegex = lib.CompileMillerRegexOrDie(SeparatorRegexFromArg(args[*pargi+1]))
|
||||
regex, err := lib.CompileMillerRegex(SeparatorRegexFromArg(args[*pargi+1]))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
options.ReaderOptions.IFSRegex = regex
|
||||
options.ReaderOptions.ifsWasSpecified = true
|
||||
}
|
||||
*pargi += 2
|
||||
|
|
@ -316,7 +324,11 @@ var SeparatorFlagSection = FlagSection{
|
|||
if err := CheckArgCount(args, *pargi, argc, 2); err != nil {
|
||||
return err
|
||||
}
|
||||
options.ReaderOptions.IPSRegex = lib.CompileMillerRegexOrDie(SeparatorRegexFromArg(args[*pargi+1]))
|
||||
regex, err := lib.CompileMillerRegex(SeparatorRegexFromArg(args[*pargi+1]))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
options.ReaderOptions.IPSRegex = regex
|
||||
options.ReaderOptions.ipsWasSpecified = true
|
||||
*pargi += 2
|
||||
return nil
|
||||
|
|
|
|||
|
|
@ -411,11 +411,12 @@ func (root *RootNode) RegisterOutputHandlerManager(
|
|||
root.outputHandlerManagers = append(root.outputHandlerManagers, outputHandlerManager)
|
||||
}
|
||||
|
||||
func (root *RootNode) ProcessEndOfStream() {
|
||||
func (root *RootNode) ProcessEndOfStream() error {
|
||||
for _, outputHandlerManager := range root.outputHandlerManagers {
|
||||
errs := outputHandlerManager.Close()
|
||||
if len(errs) != 0 {
|
||||
for _, err := range errs {
|
||||
// Print any additional errors here; return the first one.
|
||||
for _, err := range errs[1:] {
|
||||
fmt.Fprintf(
|
||||
os.Stderr,
|
||||
"%s: error on end-of-stream close: %v\n",
|
||||
|
|
@ -423,9 +424,10 @@ func (root *RootNode) ProcessEndOfStream() {
|
|||
err,
|
||||
)
|
||||
}
|
||||
os.Exit(1)
|
||||
return fmt.Errorf("mlr: error on end-of-stream close: %v", errs[0])
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (root *RootNode) ExecuteBeginBlocks(state *runtime.State) error {
|
||||
|
|
|
|||
|
|
@ -615,7 +615,9 @@ func (tr *TransformerPut) Transform(
|
|||
|
||||
// Send all registered OutputHandlerManager instances the end-of-stream
|
||||
// indicator.
|
||||
tr.cstRootNode.ProcessEndOfStream()
|
||||
if err := tr.cstRootNode.ProcessEndOfStream(); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
*outputRecordsAndContexts = append(*outputRecordsAndContexts, types.NewEndOfStreamMarker(&context))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -140,6 +140,52 @@ asserts on both; see also recent exit-code fixes #2171, #2146). `--errors-json`
|
|||
categorization must keep working — and gains coverage, since stream-time errors that
|
||||
today bypass `EmitStructuredError` will now arrive at the entrypoint as errors.
|
||||
|
||||
## Status (2026-07)
|
||||
|
||||
- Phase 1: done — #2198.
|
||||
- Phase 2: done — #2202.
|
||||
- Phase 3: done — #2204. Additionally, the `RecordTransformerFunc` internal
|
||||
dispatch methods were converted along with `Transform`, and the step verb's
|
||||
`tStepperAllocator` table gained an error return.
|
||||
- Phase 4: done — #2205. Went one step further than planned: the
|
||||
`auxents`/`terminals` dispatchers themselves now return exit codes instead
|
||||
of calling `os.Exit`, so `entrypoint.Main` is the single exit point below
|
||||
`main`. Also converted in the phase-5 wrap-up PR: `RootNode.ProcessEndOfStream`
|
||||
(a phase-3 leftover on the put/filter Transform path) returns an error.
|
||||
- Phase 5: **deferred** — see below.
|
||||
|
||||
### Phase 5 deferral rationale
|
||||
|
||||
The sketch below (error-Mlrvals via `FromErrorString`) turns out to have costs
|
||||
not visible at planning time: 58 regression cases pin the current
|
||||
loud-failure behavior (exact stderr message plus exit 1 via `should-fail`),
|
||||
and error-Mlrvals display as bare `(error)` — so HOF/UDF misuse would fail
|
||||
*silently* with exit 0 unless `--fail-on-data-error` is set, a debugging-UX
|
||||
regression. An alternative mechanism (a typed panic recovered at the four
|
||||
`Execute*` boundaries, then riding the phase-3 error path) would preserve
|
||||
behavior exactly, but introduces panic/recover to a codebase that has none.
|
||||
Rather than pick under time pressure, the DSL cluster is deferred to the
|
||||
issue-440 strict-mode design, where the fatal-vs-data error taxonomy has to
|
||||
be decided anyway. The remaining sites are all in `pkg/dsl/cst`: `hofs.go`
|
||||
(14), `udf.go` (8), `evaluable.go` (1), plus `builtin_function_manager.go`'s
|
||||
init-time duplicate-name assertion.
|
||||
|
||||
### Final keep-list (intentional, documented)
|
||||
|
||||
- `pkg/entrypoint/entrypoint.go` `exitOnError` — the sanctioned exit point.
|
||||
- `pkg/lib/logger.go` `InternalCodingErrorIf` family — should-never-happen
|
||||
assertions (`MLR_PANIC_ON_INTERNAL_ERROR` gives stack traces).
|
||||
- `pkg/bifs/types.go` `assertingCommon` — the `asserting_*` DSL builtins,
|
||||
whose documented contract is abort-on-failure.
|
||||
- `pkg/mlrval/mlrval_get.go` (`GetNumericToFloatValueOrDie`,
|
||||
`StrictModeCheck`) and `mlrval_output.go` (`String()` path) — inside
|
||||
`fmt.Stringer` and hot-path value accessors; error returns don't fit the
|
||||
signatures. Candidates for the 440 redesign.
|
||||
- `pkg/lib/mlrmath.go` (eigensolver non-convergence), `pkg/lib/regex.go`
|
||||
`CompileMillerRegexOrDie` (remaining callers are `lib`-internal and `bifs`
|
||||
hot paths, both on the `Evaluate`-shaped signatures deferred with phase 5;
|
||||
the `option_parse` callers were converted in the wrap-up PR).
|
||||
|
||||
## Phases (one PR each, each green under `make dev` + `make lint`)
|
||||
|
||||
### Phase 1 — top level + all mechanical swaps (clusters 2, 3a, 5, 7 partial)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue