-
Notifications
You must be signed in to change notification settings - Fork 2
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
Allow user to inherit on a per-variable basis #5
Comments
Hello @epage ,
Can you be more specific about let env = Environment::empty()
.inherit_var("PATH")
.inherit_vars_matching("CARGO_.*")
.insert("FOO", "BAR"); let env = Environment::inherit_matching("CARGO_.*")
.inherit_var("PATH")
.insert("FOO", "BAR"); |
Just a trade off to make in a fluent API. Do you keep the entire API flat or do you give it some hierarchy to make things easier to browse (in docs or auto-completion) let env = Environment::empty()
.inherit_var("PATH")
.var_append("PATH", ";path")
.insert("FOO", "BAR");
let env = Environment::inherit()
.insert("FOO", "BAR");
.remove("FOO", "BAR"); or // Environment::var(self) -> Var
// Var::inherit(self) -> Environment
// Var::append(self, value) -> Environment
// Var::remove(self) -> Environment
// Var::insert(self, value) -> Environment
let env = Environment::empty()
.var("PATH").inherit()
.var("PATH").append(";path")
.var("FOO").insert("BAR");
let env = Environment::inherit()
.var("FOO").insert("BAR");
.var("FOO").remove("BAR"); The downside to this is if you have other inherit methods ( I took this approach in assert_cli for stdin/stdout. My main motivation was to make it easier to reuse code. |
I like the fluent API you are describing but I don't understand the What's about the |
Note: one downside to the API I described is that rustfmt does not, by default, allow multiple
Yes. The use case is for variables that contain lists, like PATH. It provides a simpler way of extending the variable.
|
maybe:
Maybe rustfmt can be configure for that ? |
|
I start implementing it in #9 |
For motivation, see assert-rs/assert_cli#51
The text was updated successfully, but these errors were encountered: