Skip to content

Simplify nil-handler guards in polyscript.go and engines/*/new.go (after #111) #112

@robbyt

Description

@robbyt

Summary

After #111 lands (relaxing WithLogHandler(nil) and WithLogger(nil) to no-op across the three engines' compiler packages), six call sites that exist solely to work around the current nil-rejection become redundant:

File:line Current pattern
polyscript.go:251-253 if cfg.handler != nil { opts = append(opts, risorMachine.WithLogHandler(cfg.handler)) }
polyscript.go:262-264 same shape, starlarkMachine.WithLogHandler
polyscript.go:273-275 same shape, extismMachine.WithLogHandler
engines/risor/new.go:72-74 if cfg.handler != nil { compilerOpts = append(..., compiler.WithLogHandler(cfg.handler)) }
engines/starlark/new.go:72-74 same shape
engines/extism/new.go:85-87 same shape

Once the underlying option is itself a no-op on nil, the guards do nothing useful — the unconditional append produces identical behavior.

Proposal

Drop all six guards. Each pair of lines collapses to a single unconditional append:

opts = append(opts, risorMachine.WithLogHandler(cfg.handler))

The cfg.handler field is plain slog.Handler whose zero value is nil; the option now handles that correctly. No tests should need updating — the existing "nil handler is fine" tests stay green either way.

Out of scope

  • Behavior change for non-nil handlers — unchanged.
  • Other defensive nil-checks elsewhere in the codebase — only the six listed above are downstream of this specific design tension.

Files

  • polyscript.go — three sites
  • engines/risor/new.go
  • engines/starlark/new.go
  • engines/extism/new.go

Depends on

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions