Skip to content

feat(task): named args in depends-on #4148

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lucascolley
Copy link
Contributor

@lucascolley lucascolley commented Jul 16, 2025

looking to close gh-3536 (eventually)

TODO:

  • raise an error when extra strings are provided to an Alias task
  • allow adding these via the CLI

@lucascolley lucascolley force-pushed the named-args-depends-on branch 2 times, most recently from d67dae5 to f5cdb3a Compare July 17, 2025 12:46
@lucascolley lucascolley force-pushed the named-args-depends-on branch from f5cdb3a to 6624b9e Compare July 17, 2025 13:36
@lucascolley lucascolley changed the title feat(tasks): WIP: named args in depends-on feat(tasks): named args in depends-on Jul 17, 2025
@lucascolley lucascolley changed the title feat(tasks): named args in depends-on feat(tasks): named args in depends-on Jul 17, 2025
@lucascolley lucascolley force-pushed the named-args-depends-on branch from 6624b9e to 25a4714 Compare July 17, 2025 15:01
@lucascolley lucascolley force-pushed the named-args-depends-on branch from 25a4714 to 9c7a42d Compare July 17, 2025 15:25
Copy link
Contributor Author

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some self-review

Comment on lines +744 to +758
Value::Array(Array::from_iter(args.iter().map(|arg| {
match arg {
DependencyArg::Positional(val) => {
Value::from(val.source().to_string())
}
DependencyArg::Named(name, val) => {
let mut table = InlineTable::new();
table.insert(
name,
Value::from(val.source().to_string()),
);
Value::InlineTable(table)
}
}
}))),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this direction (Vec<DependencyArg> -> table) is currently untested for named args, I think

Comment on lines +218 to +221
let typed_dep_args = args
.iter()
.map(|a| TypedDependencyArg::Positional(a.to_string()))
.collect();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just constructing positional args for now — parsing named args from the CLI is TODO

@lucascolley lucascolley marked this pull request as ready for review July 17, 2025 15:25
Comment on lines +57 to +60
pub enum DependencyArg {
Positional(TemplateString),
Named(String, TemplateString),
}
Copy link
Contributor Author

@lucascolley lucascolley Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to allow templating on the name of the argument? If so, this would be Named(TemplateString, TemplateString). Could be left for a follow-up?

@lucascolley lucascolley changed the title feat(tasks): named args in depends-on feat(task): named args in depends-on Jul 17, 2025
Comment on lines +77 to +78
impl<'de> toml_span::Deserialize<'de> for DependencyArg {
fn deserialize(value: &mut Value<'de>) -> Result<Self, DeserError> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could do with some testing too

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

Successfully merging this pull request may close these issues.

Allow named args in depends-on
1 participant