-
Notifications
You must be signed in to change notification settings - Fork 510
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
Question: How to forward *args? #1988
Comments
Unfortunately this currently isn't possible. The best way to support this would by some kind of splat syntax: run *args:
echo "$1, $2,..."
do *args: (run *args) |
Whoops, didn't mean to close. |
A work-around today might be to call back into self.
|
This currently makes it impossible to use more complicated arguments to pass shell arguments safely to a dependency (i.e., without extra word-splitting or interpolation). I understand why arguments must be immediately stringified, because the Just data model consists of just I'd be interested in prototyping that approach if there will be interest in merging it. Sketch of such a data structure that could replace struct Value {
/// String representation of the value.
stringy: Box<str>,
/// Optionally, array representation of this value.
items: Option<Box[Box<str>]>,
}
impl Value {
pub fn new(s: String) -> Self {
Self { stringy: s, items: None }
}
pub fn from_vec(items: Vec<Box<str>>) -> Self {
Self { stringy: items.join(" "), items: Some(items.into()) }
}
}
impl Deref<str> for Value { ... } // convenience compatibility with String Just's values are immutable, so the more compact Single-value strings Once that is done, a dependency splat syntax pub(crate) fn evaluate_expression(
&mut self,
expression: &Expression<'src>,
) -> RunResult<'src, Value> {
match expression {
...
Expression::Splat => Err(Error::SplatNotAllowedInSingleValueContext),
}
}
pub(crate) fn evaluate_list_expression(
&mut self,
expression: &Expression<'src>,
) -> RunResult<'src, Vec<Value>> {
match expression {
Expression::Splat(inner) => match self.evaluate_expression(inner)? {
Value { items: Some(items), .. } => Ok(items.map(s => s.into()).collect()),
value => Ok(vec![value]),
},
other => Ok(vec![self.evaluate_expression(expr)?]),
}
} Then in let arguments = arguments
.iter()
- .map(|argument| evaluator.evaluate_expression(argument))
+ .flat_map(|argument| evaluator.evaluate_list_expression(argument))
.collect::<RunResult<Vec<Value>>>()?;
Self::run_recipe(&arguments, context, ran, recipe, true)?; The advantage of this approach is that it would be fully backwards-compatible: variadic recipe parameters will continue to be joined, unless an explicit splat expression is used. The downside is that it will complicate the Just data model by introducing strings-that-are-not-just-strings and a list context in which expressions can use different operators. A more conventional approach would be an In any case, such a dependency splat syntax would conflict with this open issue that suggests running a recipe for each splatted argument: |
@latk I definitely agree with the idea and the approach. I think in practice, you could just do: struct Value {
items: Vec<String>,
} And then There's also #2458, which would make all values lists of strings, with single strings just being a list with one item, and using this pervasively. I think this is a good idea. So yeah, I would love to see this happen, and starting with making variadic arguments nicer would be great. We should think about how to forward args and how to dispatch variadic arguments to multiple dependencies with splats at the same time, and make sure that we have syntax that works for both. Later, we can think about #2458, which would allow users to actually manipulate lists in more useful ways. All that gets easier once the codebase is converted from using strings to some kind of |
Ok. I've read through that context and will take a jab at transitioning the data model to a new I agree that using something like a Once we have list-typed values and lossless forwarding of variadic arguments to dependencies, the one feature that I really want is a list-aware |
Sounds good! I agree a first pass that just introduces the new |
In support of: * casey#2458 * casey#1988 A `Val` is a list of strings, but at each point of usage we must decide whether to view it as a singular joined string or as a list of its parts. Previously, Just values were single strings, so in most places we have to invoke `to_joined()` in order to maintain compatibility. In particular, recipes, functions, and operators like `+` or `/` operate solely on strings. This includes logical operators like `&&`, which continue to be defined on strings. That means, the values `[]` and `['']` are currently completely equivalent. So far, this is a purely internal change without externally visible effects. Only the `Bindings`/`Scope`/`Evaluator` had API changes. No new syntax is implemented. However, in expectation of expressions that build lists, a new `evaluate_list_expression() -> Vec<String>` method is introduced that could be used to implement splat or generator expressions. It is already referenced wherever we have lists of arguments, e.g. variadic functions like `shell()` and dependency arguments. But because singular expressions are equivalent to a joined string, this is an unobservable detail for now. For experimenting with lists of strings, variadic recipe parameters like `*args` now produce a multi-part Val, and default to an empty list (not a list with an empty string). Because all operators use `to_joined()`, this is an unobservable implementation detail. However, if any operator becomes list-aware, then this detail should be reverted, or moved behind an unstable setting. For better understanding of the current behavior, I added a bunch of tests. These will help detect regressions if functions or operators become list-aware. No existing tests had to be touched. Next steps: This change is just the foundation for other work, but some ideas are mutually exclusive. Relevant aspects: * list syntax in casey#2458 * list aware operators in casey#2458 * lossless forwarding of variadics: casey#1988 * invoking dependencies multiple times: casey#558 The preparatory work like `evaluate_list_expression()` is biased towards implementing a splat operator that would enable casey#2458 list syntax and casey#1988 list forwarding, but doesn't commit us to any particular design yet.
When I have
How can I forward arguments properly such that $2 is
b
injust do a b c
?The text was updated successfully, but these errors were encountered: