-
-
Notifications
You must be signed in to change notification settings - Fork 765
feat(tasks): add usage args to Tera context in run scripts
#7041
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new experimental setting task.disable_spec_from_run_scripts that allows users to opt out of parsing task run scripts for usage specifications. When enabled, the usage spec will be derived solely from the usage field, bypassing template processing of arg(), option(), or flag() calls in run scripts. This restores previous behavior and improves performance by avoiding unnecessary template rendering.
Key Changes:
- Added conditional logic in
TaskScriptParserto skip spec collection from run scripts when the setting is enabled - Introduced a new
make_usage_ctxhelper to populate template context with parsed usage values - Added comprehensive tests to verify the new setting and usage context functionality
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/task/task_script_parser.rs | Modified spec collection methods to check the new setting and conditionally skip template-based spec parsing; added helper function for usage context creation and tests |
| settings.toml | Added configuration entry for the new disable_spec_from_run_scripts experimental option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Should be ready to merge |
|
bugbot run |
|
this needs e2e tests |
…ipts Introduced a new setting `task.disable_spec_from_run_scripts` in `settings.toml` to allow users to opt out of parsing task run scripts for usage specifications. When enabled, the usage spec will be derived solely from the `usage` field, ignoring any `arg()`, `option()`, or `flag()` templates in run scripts. This change aims to restore previous behavior and improve performance by avoiding unnecessary template processing. Updated the `TaskScriptParser` to respect this new setting during spec collection, ensuring that the behavior aligns with user preferences. Added tests to verify the functionality of the new option. Related to jdx#6766
usage args to Tera context in run scripts
|
bugbot run |
src/task/task_script_parser.rs
Outdated
| let name = flag.name.to_snake_case(); | ||
| let value = if flag.var { | ||
| // Option-like flags with values | ||
| tera::Value::String(flag.default.clone().unwrap_or_default()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Variadic value flags get inconsistent types during parsing
The make_usage_ctx_from_spec_defaults function creates a String for variadic flags (flag.var = true), rather than an Array. This is inconsistent with how variadic arguments are handled and with make_usage_ctx. As a result, templates expecting an array for these flags will encounter type mismatches during spec parsing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jdx This has revealed a bug/underspecification in usage.
I am punting it for now for the sake of getting this merged. Further discussion in jdx/usage#387
No point in testing wrong behavior
### 🚀 Features - **(config)** add support for netrc by @RobotSupervisor in [#7164](#7164) - **(lock)** add resolve_lock_info to core backends for checksum fetching by @jdx in [#7180](#7180) - **(ruby)** Install ruby from a zip file over HTTPS by @KaanYT in [#7167](#7167) - **(tasks)** add `usage` args to Tera context in run scripts by @iamkroot in [#7041](#7041) ### 🐛 Bug Fixes - **(lock)** validate platform qualifiers when reading from lockfile by @jdx in [#7181](#7181) - **(task)** retry shebang scripts on ETXTBUSY by @iamkroot in [#7162](#7162) - **(ui)** remove duplicate 'mise' prefix in verbose footer output by @jdx in [#7174](#7174) ### 📦️ Dependency Updates - bump usage-lib to 2.9.0 by @jdx in [#7177](#7177) ### 📦 Registry - remove duplicated ubi and github backends from gping by @risu729 in [#7144](#7144) - disable bashly test (not working in CI) by @jdx in [#7173](#7173) - disable cfn-lint test (failing in CI) by @jdx in [#7176](#7176) ### Chore - add fd to mise.toml by @blampe in [#7178](#7178) ### New Contributors - @RobotSupervisor made their first contribution in [#7164](#7164) ## 📦 Aqua Registry Updates #### New Packages (2) - [`Kitware/CMake`](https://github.com/Kitware/CMake) - [`quarto-dev/quarto-cli`](https://github.com/quarto-dev/quarto-cli) #### Updated Packages (6) - [`apache/jena`](https://github.com/apache/jena) - [`apache/spark`](https://github.com/apache/spark) - [`danielfoehrKn/kubeswitch`](https://github.com/danielfoehrKn/kubeswitch) - [`danielfoehrKn/kubeswitch/switch-sh`](https://github.com/danielfoehrKn/kubeswitch/switch-sh) - [`evilmartians/lefthook`](https://github.com/evilmartians/lefthook) - [`updatecli/updatecli`](https://github.com/updatecli/updatecli)
We inject a
usagemap into the Tera context so that the args/flags can be accessed like{{usage.foo}}.The map is also injected during (deprecated) spec-parsing so that this phase essentially becomes a no-op.
Related to #6766
Note
Expose parsed task args/flags as a
usagemap in Tera run scripts (including defaults during spec parsing), plus docs and tests.usagemap into Tera context for run scripts using parsed args/flags (parse_run_scripts_with_args).usagemap fromusage::Specto allow{{ usage.* }}without errors.make_usage_ctx(snake_case keys, booleans/strings/arrays) andmake_usage_ctx_from_spec_defaults(handles var/count/boolean defaults).e2e/tasks/test_task_usage_map_teracovering basic flags, variadic args arrays, and hyphenated names ->snake_case.usagemap exposure and MultiString handling intask_script_parser.rs.docs/tasks/task-arguments.mdanddocs/templates.mdto document theusagemap in Tera scripts, key naming (snake_case or bracket access), value types, and examples.Written by Cursor Bugbot for commit 80cb98f. This will update automatically on new commits. Configure here.