Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inherit variables from environment #149

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ulyssessouza
Copy link

@ulyssessouza ulyssessouza commented Jul 19, 2021

This adds mirror functions to resolve inherited variables
with a lookup function.

@ulyssessouza ulyssessouza changed the title Inherit variables Inherit variables from environment Jul 19, 2021
@ulyssessouza ulyssessouza marked this pull request as draft July 19, 2021 14:51
This adds mirror functions to resolve inherited variables
with a lookup function.

Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
go.mod Outdated
@@ -1,3 +1,3 @@
module github.com/joho/godotenv
module github.com/ulyssessouza/godotenv
Copy link

Choose a reason for hiding this comment

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

This probably shouldn’t be in this PR, right?

godotenv.go Outdated
//
// It's important to note that it WILL NOT OVERRIDE an env variable that already exists - consider the .env file to set dev vars or sensible defaults
func Load(filenames ...string) (err error) {
return LoadWithLookupFn(func(string) (string, bool){

Choose a reason for hiding this comment

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

Should this one have used noLookupFn as well?

But perhaps instead of a dummy function, pass nil as lookup-function, then where it's executed, check if it's nil and if so, skip executing?

godotenv.go Show resolved Hide resolved
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
if len(line) == 0 {
err = errors.New("zero length string")
return
}
if lookupFn == nil {
lookupFn = noLookupFn
}

// ditch the comments (but keep quoted hashes)
if strings.Contains(line, "#") {

Choose a reason for hiding this comment

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

This was already there, but looks tricky (thinking of things like ${FOO##something}) https://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html


key = exportRegex.ReplaceAllString(splitString[0], "$1")
if len(splitString) != 2 {
err = errors.New("Can't separate key from value")

Choose a reason for hiding this comment

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

errors should not start with a capital; perhaps change "Can't" to "failed to"


// Parse the value
value = parseValue(splitString[1], envMap)
return
}

func validateVariableName(key string) error {

Choose a reason for hiding this comment

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

Did we actually need a validation for this? Note that there's no strict rules for variable names; this regex would also exclude Windows %VARIABLE%.

We made this mistake in the Docker Engine, and had to revert; moby/moby#16608

Generally, I would consider the variable name to be:

  • "anything" before the = (if present)
  • except when starting with # (comment)
  • and white space stripped

To be a "valid" variable.

Code that consumes this library can decide what to do with those (if anything); eg for docker we pass any variable to the runtime, and have the kernel/shell decide if it's accepted.

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.

None yet

3 participants