os: Getenv documentation update to point user to syscall.Getenv #9676
Comments
|
I'm reluctant to point people to the syscall package. I note that syscall.Getenv isn't even documented. os.Getenv used to return two values, but we dropped that because we found that 99% of the time the second value was ignored. I don't think we should change back. Perhaps we should add a function to os that returns two values. Or one that returns the environment in the form of a map. Or perhaps it's not worth worrying about. |
|
I agree that we should not point people to the syscall package and if anything we could add a new function to the os package. I've written my own os.Getenv replacements a few times so I can determine between existence and blank. |
|
I missed the lack of documentation in syscall -- my fault! I didn't realize that Getenv used to return two values, thanks for clarifying @ianlancetaylor. I think either of your two first suggestions are nice. In my head os.Environ returns a map (similar to %ENV in perl and probably others), but that's in stone. Perhaps? :
I'd be happy to take a stab at either/both solutions if they sound good. |
|
@Stantheman I will test your implementation on windows, if you like. FYI windows also distinguish between "this environment variable is blank" and "this environment variable does not exist". Alex |
|
@robpike for API design. |
|
Suggest, for os package: // LookupEnv returns the value of the environment variable with the given name. |
|
CL https://golang.org/cl/9741 mentions this issue. |
Quick version: I think it would be valuable to add a line of documentation to os.Getenv pointing the user to syscall.Getenv, if they want to check for the existence of an environment variable. I'd be happy to whip up the patch, or if someone is already in there, they can (if it's agreeable).
Long version: It looks like os.Getenv is a muddier version of syscall.Getenv. Specifically, it's syscall.Getenv, with an overloaded return that collides with possible real-world input. At first glance in the OS package, it looks like the only alternative for detecting the existence of an environment variable is to call os.Environ, but since it returns an array of strings as "KEY=VALUE" instead of a map, the user has to parse the results.
All os.Getenv does is call syscall.Getenv and throw away a value. The docs don't make it clear that syscall.Getenv would be a way better alternative to os.Environ, and it's unlikely the user would look up a (seemingly) unrelated package for a better option. The Go v1 release notes point the user to the right alternative. I think we should add a line to the docs to help point the user in the right direction, since the user will be less likely to have read those announcement notes as time goes on :).
The os.Getenv return has to stay the same for backwards-compatibility now, but I'll also argue for that to change in 2.0. That's for a separate issue though :)
The text was updated successfully, but these errors were encountered: