-
-
Notifications
You must be signed in to change notification settings - Fork 769
fix(aqua): skip whitespace before pipe token in template parser #7069
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 fixes a bug in the aqua template parser where whitespace before pipe operators caused template expressions to be parsed incorrectly, preventing tools like gradle from being installed to the correct location.
- Added
skip_whitespace()calls in the template parser'sparse_pipefunction to properly handle whitespace before pipe tokens - Added unit tests to verify the gradle template pattern works correctly
- Added an e2e test to verify the fix works end-to-end
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| e2e/backend/test_aqua | Added e2e test for gradle to verify pipe expression parsing with whitespace |
| crates/aqua-registry/src/types.rs | Added unit test for gradle package src template rendering |
| crates/aqua-registry/src/template.rs | Fixed parser to skip whitespace before checking for pipe tokens and added unit test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9eb9309 to
653afee
Compare
The template parser was not properly handling whitespace between
variables and pipe operators. When parsing a template like:
`{{.AssetWithoutExt | trimSuffix "-bin"}}`, the parser would stop
after parsing the variable because it didn't skip whitespace before
checking for the pipe token.
This caused the trimSuffix function to not be applied, resulting in
tools like gradle being installed but their binaries being inaccessible
because the path was incorrectly computed (e.g., `gradle-8.14.3-bin/bin/gradle`
instead of `gradle-8.14.3/bin/gradle`).
Fixes #7068
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
653afee to
d0afc2a
Compare
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.11.8 x -- echo |
18.5 ± 0.5 | 17.9 | 24.7 | 1.00 |
mise x -- echo |
18.7 ± 0.4 | 18.1 | 20.1 | 1.01 ± 0.03 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.11.8 env |
18.0 ± 0.3 | 17.5 | 19.0 | 1.00 |
mise env |
18.4 ± 2.2 | 17.7 | 64.6 | 1.03 ± 0.12 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.11.8 hook-env |
18.1 ± 0.3 | 17.6 | 19.8 | 1.00 |
mise hook-env |
18.4 ± 0.4 | 17.7 | 21.4 | 1.01 ± 0.03 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.11.8 ls |
15.7 ± 0.3 | 15.1 | 17.1 | 1.00 |
mise ls |
15.9 ± 0.3 | 15.3 | 17.2 | 1.01 ± 0.03 |
xtasks/test/perf
| Command | mise-2025.11.8 | mise | Variance |
|---|---|---|---|
| install (cached) | 107ms | 108ms | +0% |
| ls (cached) | 64ms | 66ms | -3% |
| bin-paths (cached) | 71ms | 71ms | +0% |
| task-ls (cached) | 423ms | 434ms | -2% |
### 📦 Registry - add charmbracelet/crush by @ev-the-dev in [#7075](#7075) ### 🚀 Features - **(aqua)** add symlink_bins option to filter exposed binaries by @jdx in [#7076](#7076) ### 🐛 Bug Fixes - **(aqua)** skip whitespace before pipe token in template parser by @jdx in [#7069](#7069) - **(docs)** link github backends to github repo URLs by @SKalt in [#7071](#7071) ### 📚 Documentation - update node examples from 22 to 24 by @jdx in [#7074](#7074) ### ⚡ Performance - **(hook-env)** add fast-path to skip initialization when nothing changed by @jdx in [#7073](#7073) ### New Contributors - @ev-the-dev made their first contribution in [#7075](#7075) - @SKalt made their first contribution in [#7071](#7071) ## 📦 Aqua Registry Updates #### New Packages (3) - [`SonarSource/sonar-scanner-cli`](https://github.com/SonarSource/sonar-scanner-cli) - [`Stranger6667/jsonschema`](https://github.com/Stranger6667/jsonschema) - [`peteretelej/tree`](https://github.com/peteretelej/tree) #### Updated Packages (2) - [`astral-sh/uv`](https://github.com/astral-sh/uv) - [`pre-commit/pre-commit`](https://github.com/pre-commit/pre-commit)
Summary
{{.AssetWithoutExt | trimSuffix "-bin"}}to be incorrectly parsed, resulting in the pipe expression being ignoredProblem
Tools that use aqua registry templates with piped functions (like gradle) were being installed but their binaries were inaccessible. For example, gradle was being installed to
gradle-8.14.3/bin/gradlebut mise was looking forgradle-8.14.3-bin/bin/gradlebecause thetrimSuffix "-bin"wasn't being applied.Root Cause
The
parse_pipefunction in the template parser didn't skip whitespace before checking for the pipe token. After parsing the primary expression (like.AssetWithoutExt), there would be whitespace in the token stream, but the parser would immediately checkmatches!(tokens.peek(), Some(Token::Pipe))which would fail because the next token wasToken::Whitespace, notToken::Pipe.Fix
Added
skip_whitespace(tokens)calls before checking for pipe tokens in theparse_pipefunction.Test plan
Fixes #7068
🤖 Generated with Claude Code
Note
Handle whitespace around pipe tokens in the template parser and add Gradle-focused unit/e2e tests to verify pipe expressions.
parse_pipeincrates/aqua-registry/src/template.rstoskip_whitespacebefore/after checking for|, enabling expressions like{{.AssetWithoutExt | trimSuffix "-bin"}}.test_gradle_src_templateintemplate.rsfor whitespace-preceded pipe usage.crates/aqua-registry/src/types.rsvalidating Gradlesrctemplate resolution.e2e/backend/test_aquato assert Gradle binary path resolution usingmise which.Written by Cursor Bugbot for commit d0afc2a. This will update automatically on new commits. Configure here.