Repository navigation
fix(interpreter): parse arithmetic parameter operators at first operator - #1472
Merged
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bashkit | 5e13c8b | Commit Preview URL | Apr 25 2026, 04:10 AM |
chaliy
force-pushed
the
2026-04-24-fix-arithmetic-parameter-parsing-issue
branch
from
April 25, 2026 04:09
9436e4d to
5e13c8b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
chaliy
added a commit
that referenced
this pull request
May 30, 2026
…tor (#1472) ### Motivation - A naive search for `"%%"`/`"##"` inside `expand_param_op_in_arithmetic` could match those sequences inside the pattern portion (e.g. `${var%foo%%bar}`), which truncated the variable name and produced incorrect arithmetic results. - The regression affects arithmetic-base expansions like `10#${var%foo%%bar}` and `10#${var#foo##bar}` and was introduced when adding operator handling for arithmetic contexts. - The goal is to detect the operator at the first operator boundary (left-to-right) so patterns containing `%%`/`##` are not misinterpreted as the operator location. ### Description - Replace the global `find`-based detection in `expand_param_op_in_arithmetic` with a left-to-right `char_indices()` scan that identifies the first operator boundary and then dispatches handling for `%%`/`%`/`##`/`#` and `:-` accordingly in `crates/bashkit/src/interpreter/mod.rs`. - Add regression unit tests exercising `${var%foo%%bar}` and `${var#foo##bar}` inside arithmetic base expressions in `mod tests` of `crates/bashkit/src/interpreter/mod.rs`. - Extend spec coverage by adding matching spec cases to `crates/bashkit/tests/spec_cases/bash/arithmetic-base-expansion.test.sh` to ensure the behavior remains covered by the spec runner. ### Testing - Ran `cargo fmt --check` which succeeded. - Ran targeted unit tests with `cargo test --features http_client` for the new tests and `arithmetic_base` scenarios, and the added tests `test_arithmetic_base_suffix_pattern_with_double_percent` and `test_arithmetic_base_prefix_pattern_with_double_hash` passed. - Ran the `arithmetic_base` test grouping and observed no regressions in existing arithmetic base tests (all executed tests passed). ------ [Codex Task](https://chatgpt.com/codex/cloud/tasks/task_e_69eadf19cfa4832ba2ad750974669892)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
"%%"/"##"insideexpand_param_op_in_arithmeticcould match those sequences inside the pattern portion (e.g.${var%foo%%bar}), which truncated the variable name and produced incorrect arithmetic results.10#${var%foo%%bar}and10#${var#foo##bar}and was introduced when adding operator handling for arithmetic contexts.%%/##are not misinterpreted as the operator location.Description
find-based detection inexpand_param_op_in_arithmeticwith a left-to-rightchar_indices()scan that identifies the first operator boundary and then dispatches handling for%%/%/##/#and:-accordingly incrates/bashkit/src/interpreter/mod.rs.${var%foo%%bar}and${var#foo##bar}inside arithmetic base expressions inmod testsofcrates/bashkit/src/interpreter/mod.rs.crates/bashkit/tests/spec_cases/bash/arithmetic-base-expansion.test.shto ensure the behavior remains covered by the spec runner.Testing
cargo fmt --checkwhich succeeded.cargo test --features http_clientfor the new tests andarithmetic_basescenarios, and the added teststest_arithmetic_base_suffix_pattern_with_double_percentandtest_arithmetic_base_prefix_pattern_with_double_hashpassed.arithmetic_basetest grouping and observed no regressions in existing arithmetic base tests (all executed tests passed).Codex Task