Skip to content

docs: note that env cannot distinguish an empty variable from an unset one - #436

Open
milanobrtlik wants to merge 1 commit into
caarlos0:mainfrom
milanobrtlik:docs/empty-value-followup
Open

milanobrtlik wants to merge 1 commit into
caarlos0:mainfrom
milanobrtlik:docs/empty-value-followup

Conversation

@milanobrtlik

Copy link
Copy Markdown

Follow-up to #432, which landed the caveat that was originally proposed in #428.

Two things are still missing:

What to do about it. getOr returns (defaultValue, exists=true, isDefault=true) for a variable that is set but empty:

case exists && value == "" && defExists:
	return defaultValue, true, true

So with an envDefault in place, the two cases are indistinguishable. A field
that needs to tell them apart has to leave the default out and either use
notEmpty, which then rejects the empty value, or check os.LookupEnv before
applying a fallback. The Caveats note now says so.

The required/notEmpty trap is asserted, but nothing checks it. The
README states it; no test covers it. Example_parseEmptyEnvFallsBackToDefault
now covers all three cases instead of the plain fallback only, so go test
verifies the claim and it shows up on pkg.go.dev.

The example's variables are renamed to EMPTY_*, since FOO is also set by
other examples in this package.

No behavior changes.

…t one

Follow-up to caarlos0#432, which documented that a variable set to an empty value
falls back to `envDefault`. Two things it left out:

- What to do about it. With an `envDefault` in place the two cases are
  indistinguishable, so a field that needs to tell them apart has to leave the
  default out and rely on `notEmpty` or `os.LookupEnv`.

- The `required`/`notEmpty` trap is stated in the README, but nothing checks
  it. The example now covers all three cases, so `go test` verifies the claim
  and pkg.go.dev shows it.

The example's variables are renamed to `EMPTY_*`, since `FOO` is also used by
other examples in this package.
@milanobrtlik

Copy link
Copy Markdown
Author

Note for whoever reviews this alongside #433: that PR adds ,notEmptyIfDefined, which is a built-in answer to the case this note describes.

The two don't conflict — this touches the Caveats block, #433 touches the env tag options list — but if #433 lands, the sentence about os.LookupEnv should gain a pointer to the new tag. Happy to update it either way, in whichever order they merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant