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 support for environment variable interopolation #264

Closed
jonringer opened this issue Nov 28, 2017 · 8 comments
Closed

Add support for environment variable interopolation #264

jonringer opened this issue Nov 28, 2017 · 8 comments

Comments

@jonringer
Copy link

If you do want to add a variable that derives from an environment variable, you have do something like:

OPENSSL_CACHE_DIR= `echo $CACHE_DIR` + "/openssl"

feels like it would be more ergonomic to allow for it to resolve the env var itself:

OPENSSL_CACHE_DIR= $CACHE_DIR + "/openssl"

Seems like the grammar should be able to support this in a non-breaking way:

ENV_VAR  =     \$[a-zA-Z_][a-zA-Z0-9_-]*

expression    : STRING
              | RAW_STRING
              | NAME
              | BACKTICK
              | ENV_VAR
              | expression '+' expression

And then when evaluating the ENV_VAR we just take the "name" portion of it and pass it to std::env::var which should be a platform agnostic way to get that value.

@jonringer
Copy link
Author

I would feel comfortable taking this endeavor on, just wondering if you think the proposed implementation would be acceptable.

@jonringer
Copy link
Author

This also seems to be related to #245 where environment variables are being resolved as well, not sure if you want to merge the two and find a way to solve both.

@casey
Copy link
Owner

casey commented Nov 30, 2017

I think there are a few solutions here:

  1. $FOO – a dollar sign and then a NAME token evaluates to the environment variable whose name is the characters in the NAME token. So, $FOO would resolve to the environment variable whose name is "FOO".

This is ergonomic and similar to shell syntax, but NAME tokens are limited, they can't start with numbers (in case that just is extended in the future to support integer literals), so it won't work in all cases.

  1. $"FOO" – a dollar sign and then a STRING or RAW_STRING evaluates to the environment variable whose name is the STRONG or RAW_STRING. So, $"FOO" would evaluate to the environment variable whose name is "FOO".

This is less ergonomic and less similar to shell syntax, and so will probably be quite surprising to users.

  1. environment-variable("FOO") – Add function-call syntax, and add a built-in function called environment-variable that takes one argument, namely an expression whose value is the name of an environment variable name to look up. (The name could be shorter, like env())

This is probably the most work, but feels like the best solution to me. Once function-call syntax is allowed, all sorts of useful built-in functions can be added without needing to modify the parser.

Also, function-call notation permits multiple arguments more naturally than $.., which seems very useful. For example, env("FOO") might evaluate to the value of the environment variable named FOO and produce an error if it wasn't present, but env("FOO", "hello") might evaluate to the value of the "FOO" environment variable or the string "hello" if it isn't present. This would also solve #245, which is nice.

The grammar would be amended like so:

expression
  : name '(' arguments? ')'
  | value '+' expression
  | value

arguments
  : expression ',' arguments
  | expression ','?

value
  : STRING
  | RAW_STRING
  | NAME
  | BACKTICK

This is a lot more work than the $... version, unfortunately. Off the top of my head it involves: adding the (, ) and , tokens, adding built-in functions, adding compilation errors for misusing functions, implementing the function itself, and changing the evaluator to evaluate functions.

@jonringer
Copy link
Author

Just some thoughts:

  1. I think the $... version holds with the Law of least suprise in which that is what you would expect in behavior. Not to mention it's a relatively easy addition to the token table and grammar. On a side note, I guess it's also kind of weird that the variable definitions are "like" bash in some respects, but different in others (bash doesnt have explicit concatenation, etc). But this method is still what I personally would prefer.

  2. The $"FOO" method seems to deviate from what you expect the most to be able to express this behavior. Especially confusing when "$FOO" is a common way to ensure that the bash string doesn't delimit on whitespace.

  3. With the function route, there's also the fact that now you have a function prelude (built-in functions). And you will have to be able to resolve the functions in your symbol table when you're parsing (a lot more complexity). This would take just from being a script construction/execution tool to a full on interpreter.
    Although I have very little invested in the project, I'm afraid the introduction of functions will be expand the complexity far greater than intended. Even on the readme, just is described as, "a handy way to save and run project-specific commands.", and I think that makes this project fit a really desired niche across a lot of different projects, but this change would make it almost into a general purpose language.

@casey
Copy link
Owner

casey commented Nov 30, 2017

The function route is definitely a bit more work, but $ is like a single-purpose function that has its own syntax, whereas function syntax can be re-used for other functionality.

Also, if $FOO worked, I would want it to abort with an error if the variable wasn't defined, in which case there would need to be a separate syntax for get-variable-with-default, whereas function syntax takes multiple arguments, and thus naturally provide get-with-default.

And, finally, I've been kind of expecting that I'd need to add function call syntax eventually, so the extra complexity isn't too a big deal :P

@jonringer
Copy link
Author

This sounds like #245 a lot, which I think would be a nice feature anyway. But I don't think instantly failing is a terrible idea, sometimes if you're in a new environment it's nice to know when you don't have something populated (PORT, target_dir, etc), in a certain light this might be a feature. Or, maybe only have it fail if the recipe will actually reference the value. Because sometimes defaults can get kind of weird, like what OS they are using.

Also, the get-with-default would be useful in a recipe, but still awkward with variables until the ability to then reference the value in another variable becomes available:

PORT ?= 8080
ADDR = "localhost:" + {{PORT}} # or something similar

You could also just do this in the recipe, but in a few cases it feels more ergonomic to construct common settings in the just prelude.

@casey
Copy link
Owner

casey commented Dec 2, 2017

Put up #277 which adds simple functions that don't take any arguments. Next step is functions that actually take arguments.

I'll probably add:

env(variable-name) – Look up variable-name in the environment, aborting if it isn't defined.
env-default(variable-name, default) – Look up variable-name in the environment, returning default if it isn't found.

Not really wedded to the names, and undecided on whether to allow functions to take a variable number of arguments or not.

These would solve the use-case in #245 like so:

export MY_ENV_VAR = env-default("MY_ENV_VAR", "foo")

@casey
Copy link
Owner

casey commented Dec 2, 2017

You can now use env_var(key) or env_var_or_default(key, default) to look up environment variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants