-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
"Environment variable" is confusing #17262
Comments
Added Area-Library, Triaged labels. |
This comment was originally written by @tatumizer I was sure it's THAT environment which you say it isn't. Wouldn't it be a good idea just to mix both types of "environment" together (command-line overriding OS environment), both in Platform and here? Otherwise you only increase confusion: everyone will wonder why these "environment" vars, no matter how you name them, are bestowed with int, bool etc conversion methods, and those are not. |
The fromEnvironment constructors will retrieve their values from the actual embedding of Dart. The documentation says: * Environment declarations are provided by the surrounding system compiling Whatever "the surrounding system" does is implementation specific, and needs to be documented for each embedding of Dart. The current implementation of both the standalone VM (dart executable) and dart2js only considers name/value pairs passed using -D options. Of cause this can be changed to also add the process environment, but I don't think that is a good idea. When this was added the name of the constructors was discussed, (e.g. fromConfiguration and fromSystem and a Java like system property) but we ended deciding on fromEnvironment. As this is part of the core library we don't want to do any breaking changes here. Added AsDesigned label. |
This comment was originally written by @tatumizer Can you reclassify it as documentation bug then? In API, just say: "so and so, not to be confused with another Environment available through Platform". |
This comment was originally written by yu.as...@gmail.com
Can you please call it “environment declaration”, “environment constant”, or something along these? I do not think that it is officially called “environment variable,” and even if it is, it should not be called “environment variable” for the exact reason you stated. |
This comment was originally written by @Andersmholmgren Arrrgghhh I just got bitten by this one too. +100 for doing command line with fallback to environment. Otherwise you really need to rename them. System properties is the term I've always heard used for the command line -D variables. But I prefer the idea of keeping the name and having the combined use of it. I think everyone seeing these like int.fromEnvironment will assume they are short cuts for Platform.environment with type conversion (which is very useful by the way). As it stands you have this conversion for command line args with methods named to suggest they are Environment Variables. |
The Dart VM and dart2js have a feature where compile-time constant values can be passed in on the command-line. They can then be accessed at runtime using new bool.fromEnvironment().
Unfortunately, the computing world already has something called "environment variables", which are unrelated to these. (See: http://en.wikipedia.org/wiki/Environment_variable.) Actual enviroment variables are supported in dart:io using Platform.environment.
Every time this unrelated feature has come up, I've watched users get confused who incorrectly think that bool.fromEnvironment() can be used to access variables from the environment. It's difficult to even discuss the feature without causing confusion. I've had more multiple conversations along the lines of:
Me: "We could pass that in using an environment variable."
Them: "How does that work if they don't use dart:io?"
Me: "No, not, like, an enviroment variable environment variable. You know, the other environment variable thing. The fromEnvironment() stuff. Not a real environment variable."
Can we change the name of these non-environment variables to something less confusing?
The text was updated successfully, but these errors were encountered: