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

proposal: os: add function to return environment var with default value #35696

Closed
timoheijne opened this issue Nov 19, 2019 · 5 comments

Comments

@timoheijne
Copy link

@timoheijne timoheijne commented Nov 19, 2019

Adding a very small QoL function to go giving the ability to return an environment variable with a possible fallback in one line

Since go doesn't support Overloading or Optional parameters (as far as I am aware)
I am proposing to add

os.GetEnvWithDefault(envname, defaultvalue)

the added benefit is this way it's also a non-breaking change

@gopherbot gopherbot added this to the Proposal milestone Nov 19, 2019
@gopherbot gopherbot added the Proposal label Nov 19, 2019
@timoheijne timoheijne changed the title proposal: os: add function to return with default value proposal: os: add function to return environment var with default value Nov 19, 2019
@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 19, 2019

// use the default if empty
val := os.Getenv(name)
if val == "" {
    val = default
}

// use the default if not set
val, ok := os.LookupEnv(name)
if !ok {
    val = default
}

Which of the two do you mean? Either seems reasonable, and I don't think there's a clear winner.

I think it's best to let the developer write another three lines and make it explicit and simple. If you want to avoid the extra typing, just set up an editor macro or copy-paste.

@timoheijne

This comment has been minimized.

Copy link
Author

@timoheijne timoheijne commented Nov 19, 2019

I would personally go with

val, ok := syscall.Getenv(key)
if !ok {
    val=default
} 

because this is technically how lookupenv works. If you look at the env.go they don't look too different from each other too much

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Nov 27, 2019

@timoheijne You missed or at least did not respond to the semantic difference that @mvdan was pointing out. Some programs will want to distinguish unset from empty; others will not. That's why both os.Getenv and os.LookupEnv exist. If you want to also set a default in one case or the other, it seems reasonable to ask the developer to write out which matters, explicitly.

@mvdan:

I think it's best to let the developer write another three lines and make it explicit and simple. If you want to avoid the extra typing, just set up an editor macro or copy-paste.

Or, you know, write a function. :-)

@rsc rsc added this to Incoming in Proposals Nov 27, 2019
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Nov 27, 2019

Given the semantic subtleties and how easy it is to write the function you want, this seems like a likely decline.

Leaving open for a week for final comments.

@rsc rsc moved this from Incoming to Likely Decline in Proposals Dec 4, 2019
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Dec 4, 2019

No change in consensus, so declining.

@rsc rsc closed this Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Likely Decline
4 participants
You can’t perform that action at this time.