Freyskeyd / environment

6 stars 2 forks source link

Allow user to inherit on a per-variable basis #5

Open epage opened 6 years ago

epage commented 6 years ago

For motivation, see https://github.com/killercup/assert_cli/issues/51

Freyskeyd commented 6 years ago

Hello @epage ,

The main question is what the API should look like. As Environment grows different var manipulation functions (inherit, append, etc), should they be var_append, inherit_var, etc or var(key).inherit(), var(key).append(text), etc?

Can you be more specific about var(key).inherit() etc?

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");
epage commented 6 years ago

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 (inherit_vars, inherit_matches) they will not longer be next to each other in the docs or in auto-completion.

I took this approach in assert_cli for stdin/stdout. My main motivation was to make it easier to reuse code. https://github.com/killercup/assert_cli/blob/master/src/assert.rs#L112

Freyskeyd commented 6 years ago

I like the fluent API you are describing but I don't understand the append is it an append on the variable's value?

What's about the insert?

epage commented 6 years ago

Note: one downside to the API I described is that rustfmt does not, by default, allow multiple .var("FOO").insert("BAR"), it will be split across lines.

append is it an append on the variable's value?

Yes. The use case is for variables that contain lists, like PATH. It provides a simpler way of extending the variable.

What's about the insert?

var(key).insert(value) was meant to be like .insert(key, value). In this nested API, it might be better to give it a different name. var(key).create(value)? Unsure. In some respects it seems weird to use var(key) to access a non-existent item but that is also how the proposed var(key).inherit() works. shug

Freyskeyd commented 6 years ago

maybe:

Maybe rustfmt can be configure for that ?

epage commented 6 years ago

equal sounds like a test to me.

assign would be more semantically clear and I kind of like.

set_to I have this weird thing about avoiding prepositions like that. shrug

Freyskeyd commented 6 years ago

I start implementing it in #9