Skip to content

fix: honor explicit empty substitution operands - #75

Merged
a8m merged 1 commit into
a8m:masterfrom
jihtsan:codex/61-empty-default
Oct 18, 2026
Merged

a8m merged 1 commit into
a8m:masterfrom
jihtsan:codex/61-empty-default

Conversation

@jihtsan

@jihtsan jihtsan commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Fixes #61.

An explicit empty operand was left as a nil node, so ${FOO:=} with -no-unset was evaluated like ${FOO} and failed. Initialize an empty text operand when the parser reads a substitution operator; a supplied operand still replaces it normally.

This covers all six existing operators without changing their documented semantics. The regression matrix covers unset, empty and non-empty variables under all four restriction modes, including preservation of -no-empty errors for ${EMPTY-} and ${EMPTY=}. Bare variables still obey restrictions.

Validation on macOS arm64:

  • New regression cases fail against the original parser and pass with the fix.
  • Go 1.25.5 (the version declared by go.mod): go test -count=1 -vet=off ./... and go test -count=1 -race -vet=off ./... pass.
  • Built CLI reproducer passes with -no-unset, with -no-unset -fail-fast, and with -no-unset -no-empty; output is value: plus a newline, exit 0.
  • Separate Go 1.27.2 harness: 158 boundary cases pass, including Quick/AllErrors, all operators/restrictions, concatenation and escaping.
  • gofmt -d parse/parse.go parse/parse_test.go and git diff --check are clean.

go test ./... with vet enabled fails on both this branch and unchanged base 795eb83 at existing non-constant format strings in cmd/envsubst/main.go:36 and :100. Those lines are unchanged; tests above explicitly disable vet and do not claim that check passes. Linux, Windows and the remaining CI toolchains were not run locally.

@a8m
a8m requested a balanced review from Copilot October 18, 2026 12:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused fix preserves existing operator semantics, with regression coverage and no unresolved findings.

No open issues 🎉

What changed in this PR

Fixes #61 by distinguishing explicit empty substitution operands from missing operands, allowing ${FOO:=} under -no-unset.

Changes:

  • Initializes an empty text operand when parsing a substitution operator.
  • Adds regression cases for all six operators across variable states and restriction modes.
File Description
parse/​parse.go Preserves explicit empty substitution operands.
parse/​parse_test.go Adds empty-operand regression coverage.

Review effort: Balanced

@a8m
a8m merged commit 8da3c87 into a8m:master Oct 18, 2026
2 checks passed
@a8m

a8m commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Thanks for the contribution, @jihtsan.

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.

envsubst -no-unset blows up on ${FOO:=}

3 participants