Support array indexing by range #628

hdwalters · 2024-12-05T08:10:03Z

Allow array variables to be indexed by range, for example:

let numbers = ["zero", "one", "two", "three"]
echo numbers[1..=2]

Required in order to implement #629.

mks-h

These are some surface level suggestions to improve the code's readability.

src/modules/variable/get.rs

src/tests/validity/array_get_range_local.ab

src/modules/variable/get.rs

Ph0enixKM

Looks good overall

src/modules/expression/expr.rs

src/modules/expression/binop/range.rs

src/modules/variable/get.rs

Ph0enixKM

Looks good

src/modules/expression/unop/neg.rs

Self reported lack of Rust knowledge to review code doesn't pair well with approvals of complex Rust PRs

mks-h

Needs tests for error messages.

src/modules/expression/unop/neg.rs

src/modules/variable/mod.rs

src/tests/errors.rs

mks-h

Great job, thanks!

hdwalters added 6 commits December 4, 2024 23:13

Use pretty assertions crate.

b4d7b24

Rename, rewrite and extend array tests.

5a1d007

Refactor variable translation; optimise imports.

faada0a

Refactor array index by negative number.

8457a11

Support array index by range.

e80a92d

Quote dollar for by ref array concatenation.

e02275a

hdwalters requested review from Ph0enixKM, mks-h and b1ek December 5, 2024 08:15

mks-h requested changes Dec 5, 2024

View reviewed changes

Ph0enixKM requested changes Dec 5, 2024

View reviewed changes

src/modules/expression/expr.rs Show resolved Hide resolved

src/modules/expression/binop/range.rs Show resolved Hide resolved

src/modules/variable/get.rs Outdated Show resolved Hide resolved

src/modules/variable/get.rs Outdated Show resolved Hide resolved

hdwalters added 2 commits December 5, 2024 21:38

Separate inclusive and exclusive range tests.

c85c3bb

Refactor array variable indexing and typing.

3f1bf5f

hdwalters requested review from mks-h and Ph0enixKM December 6, 2024 07:27

hdwalters added 2 commits December 6, 2024 07:35

Address Clippy errors.

0a4e704

Add comment to integer indexing test.

7b649ef

Ph0enixKM approved these changes Dec 9, 2024

View reviewed changes

src/modules/expression/unop/neg.rs Outdated Show resolved Hide resolved

Call standard translate function if negative is not numeric.

c87ed9e

Mte90 previously approved these changes Dec 10, 2024

View reviewed changes

mks-h requested changes Dec 10, 2024

View reviewed changes

src/modules/expression/unop/neg.rs Show resolved Hide resolved

src/modules/variable/mod.rs Outdated Show resolved Hide resolved

Add tests for incorrect array slicing.

4a17cb4

mks-h reviewed Dec 11, 2024

View reviewed changes

src/tests/errors.rs Outdated Show resolved Hide resolved

hdwalters added 2 commits December 11, 2024 22:32

Reword error for incorrect array slicing.

7b5bd3d

Reword error for incorrect array slicing.

ebeb154

Mte90 requested a review from mks-h December 12, 2024 09:26

Reword error for incorrect array slicing.

61239a8

mks-h approved these changes Dec 12, 2024

View reviewed changes

hdwalters merged commit a4bbf9e into amber-lang:master Dec 12, 2024
1 check passed

lens0021 mentioned this pull request Dec 12, 2024

test: Use the bash container and a matrix strategy for bash versions #616

Open

hdwalters deleted the array-indexing-by-range branch December 12, 2024 18:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support array indexing by range #628

Support array indexing by range #628

hdwalters commented Dec 5, 2024 •

edited

Loading

mks-h left a comment

Ph0enixKM left a comment

Ph0enixKM left a comment

mks-h left a comment

mks-h left a comment

Support array indexing by range #628

Support array indexing by range #628

Conversation

hdwalters commented Dec 5, 2024 • edited Loading

mks-h left a comment

Choose a reason for hiding this comment

Ph0enixKM left a comment

Choose a reason for hiding this comment

Ph0enixKM left a comment

Choose a reason for hiding this comment

mks-h left a comment

Choose a reason for hiding this comment

mks-h left a comment

Choose a reason for hiding this comment

hdwalters commented Dec 5, 2024 •

edited

Loading