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

Add partial version of getEnv #443

Closed
kaoskorobase opened this issue Apr 26, 2016 · 5 comments · Fixed by #712
Closed

Add partial version of getEnv #443

kaoskorobase opened this issue Apr 26, 2016 · 5 comments · Fixed by #712

Comments

@kaoskorobase
Copy link
Contributor

This is a suggestion to add a partial version of getEnv, because I end up defining it in almost every shake build system; maybe others, too?

Something like

getEnv_ :: String -> Action String
getEnv_ name =
  getEnvWithDefault
    (error $ "Environment variable " ++ name ++ " is undefined")
    name

Where error probably should be replaced by a more suitable error signalling mechanism.

@ndmitchell
Copy link
Owner

It's somewhat annoying - there is a perfect name for a partial version of getEnv and it is exactly getEnv just like in the base library. And there's a great name for getEnv too, which is lookupEnv.

However, for now I suggest just naming this one getEnvError - does that seem reasonable?

@kaoskorobase
Copy link
Contributor Author

Why not follow Shake's convention of adding a tick to Action versions of library functions with the same name? So there could be getEnv', corresponding to System.Environment.getEnv and lookupEnv' corresponding to System.Environment.lookupEnv (aliased by getEnv). In a later release getEnv could just be removed.

@ndmitchell
Copy link
Owner

Shake has many conventions - way too many conventions. There's readFile' for the Action version of that, doesDirectoryExist for the Action version of that... It's way too inconsistent, and I'm not even sure what my preferred approach is.

@kaoskorobase
Copy link
Contributor Author

Flip a coin? ;-)

Anyway, I'd be happy to see getEnvError in Shake regardless of what it's called! A minor convenience of the tick approach is that you can import Shake and base modules without qualifying imports.

@ndmitchell
Copy link
Owner

Rather than flip a coin I thought I'd solicit opinions: https://groups.google.com/d/msg/shake-build-system/8ZWQr2g3XM0/Z_hpQaEFAwAJ

Should that not work, I'll flip a coin :)

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 a pull request may close this issue.

2 participants