Add notempty struct tag

[HalloTschuess]

Currently there is no easy way of knowing if a environment variable is set with an empty value except for checking the value yourself.

In certain workflows like for example when using docker-compose with variable substituion this can be a problem.
In this case a missing system variable is replaced by an empty string. The resulting container sees a set but empty variable. Therefore the required keyword does not result in an error and is useless.

notempty uses required and errors on a empty variable with message "empty value"

[sethvargo]

Sorry, I don't really understand the use case here. required looks at whether an envvar is set; it does not look at whether the value is the "zero" value.

To be honest, this feels like a bug in docker-compose. An unset environment variable and an environment variable set to the empty string are intentionally different constructs.

[HalloTschuess]

Ok maybe my use case is too specific but it was just an example. The behavior of docker-compose is actually not a bug but probably not taking full adavatage of docker-compose features. Unfortunately I currently have no easy way of solving this.

Nontheless I still think checking for zero value in genaral is a valid use case. There could be multible different reasons why an variable is set empty by accident. A few I can think of right now are for example a misbehaving setup scripts or you forgotten value in a .env files, possibly even a misconfigured system.

While values which are converted into a different type might error straigt away string values will be empty. On fields you know that can not be empty like for example a database connection string you still have to check. This can be cumbersome for large structs and might negate the gains from using this lib in the first place.
Sure an empty value might be noticed immediatly because for example a connection failed, but the error might not be obvious or it does fail much later or not at all depending on what the value is used for.

[sethvargo]

All of the examples you described are upstream bugs (misbehaving setup scripts, forgotten values in .env, misconfigured system). I don't think it's the responsibility of a library to handle those misconfigurations.

This library isn't a replacement for configuration validation - you still need to do that. For example, you still need to make sure the value given for a database string actually looks like a database string.

If you really wanted this functionality, you could implement a custom type and decoder:

type AlwaysString string

func (a AlwaysString) EnvDecode(val string) error {
  if val == "" {
    return fmt.Errorf("...")
  }
  *a = val
  return nil
}

But I don't think this makes sense as a general-purpose functionality in the library.

[HalloTschuess]

Well ok but what is required then for? This also a kind of validation which you only need when you have a so called upstream bug. Why even error out when type conversion does not work. I mean its an upstream bug just make sure you provide the correct value. Why do you need default values? You could just check the value yourself.

What I'm getting at here is that I think checking for an empty environment variable is well whithin scope of a library like this. (Btw similar libs support this feature). It's not like fully fletched regex-validatation or doing fancy log4j-style bloat.
You can even see how little had to be changed for this feature so it can't be out of scope that much.

I really like the style and simplicity (like prefixes and you only use one struct tag) of this library and would hate to not use it.

And to be honest writing notempty is way cleaner than having to check multiple values within your main (or where ever you want to check). I just gave some quick examples but there are many cases where empty values aren't that easy to spot.

[sethvargo]

required is for when a value must be specified in the environment. There's a fundamental difference between:

FOO=

and

unset FOO

In the first, FOO is set. It's set to an empty value, but it's set. Thus, it would pass a required validation. In the second, FOO is unset. That variable is undefined. Thus, it would fail a required validation.

What I'm getting at here is that I think checking for an empty environment variable is well whithin scope of a library like this. (Btw similar libs support this feature).

I disagree. Feel free to use those other libs.

I really like the style and simplicity (like prefixes and you only use one struct tag) of this library and would hate to not use it.

This library is small and simple because I reject changes like these.

[HalloTschuess]

Even Bash (and most programs I know of) treats empty and unset variables mostly the same. There is not that much difference. It's by far not out of scope.

[sethvargo]

Even Bash [...] treats empty and unset variables mostly the same.

This is factually incorrect 1, 2, 3.

[HalloTschuess]

Yes of course thats why I said mostly. I have yet to actually differenciate between empty and unset in bash. In your provided links even the easiest way of checking for unset is checking if the variable is zero length aka empty

[HalloTschuess]

As I probably cannot convince you otherwise I would like to know why you think validating if a variable is set with required is in scope of this libray or if you think required was a mistake.

[HalloTschuess]

Hey, so I realize my last few comments sounded a bit sour, and I apologize for that. It's just a bit frustrating not to understand the issue because to me your arguments and existing code are somewhat contradicting. Yes I am a relatively novice programmer, and I'm genuinely just trying to learn and improve. With your employment history you are probably very experienced, and I don't and never wanted to undermine that fact.

So trying to clear things up:

What I do understand:

• we might never fully agree and this PR probably will never get merged

• the issue of not wanting to bloat a simple library, in this case more or less complex validation
  There are already plenty good (struct) validation libs. But these are way more powerful (e.g. regex validation). These kinds of validation are way out of scope which is a totally valid point.

• the difference between unset/undefined vs ""
  Obviously these two are not the same and most programming languages handle them accordingly (Go as a small exception as the zero value of a string is "")

  My point was that in many (not all) cases (programs, scripts) I have come across unset and "" in the realm of environment variables are treated in similar ways -> missing/empty? -> error (or default). I won't deny this could be lack of experience.
  But thinking about it notempty might even in a way conflict with the current implementation of default values.

What I cannot wrap my head around:

• not wanting to deal with misconfigurations by arguing not being a validation lib even though there are some kinds of validation already built in

  All of the examples you described are upstream bugs (misbehaving setup scripts, forgotten values in .env, misconfigured system). I don't think it's the responsibility of a library to handle those misconfigurations.

  This might be challenged by required and all kinds of parsing. These return errors and therefore validate a value in some shape or form. Even pretty complex structures are validatable this way either by built in parsers or custom types.
  I think it is simply misleading to argue handling misconfigurations are not scope of this library when in fact some kinds are handled anyway.

  In a theoretical world where this lib had no required and none of the parsers returned errors I would totally agree with you. But let's be honest that lib would be pretty useless.

  You could even argue required is redundant as os.LookupEnv all you need. But then why use such a library in first place, right?

Summary: I don't understand the general difference between notempty, required and parser errors as those are all validating upstream bugs / misconfiguration. Especially when in my experience many programs I know of treat unset and "" similarly.

I am grateful for any response helping me to resolve my confusion.

Sorry for the wall of text 🍪