`slice` should handle negative numbers

[epage]

Example test:

    assert_eq!(v!("ar"), filters!(slice, v!("foobar"), v!(-2), v!(2)));

• Rust implementation
• Ruby implementation
• Relevant tests
  • Once this is working, ideally the test will start failing. We would just need to remove the #[should_panic] on it
  • If the test doesn't start failing, please temporarily remove #[should_panic], and find or create an issue for the new failure

[ryanyz10]

If this hasn't been fixed yet, I'd like to try fixing it. Is the relevant code insrc/filters/std/slice.rs?

[epage]

Yes it is (sorry for the out of date link in the description).

Thanks for the help! Whats interesting is it looks like we have some form of negative indexing support. I do not remember why this was failing though.

[ryanyz10]

Yeah, it looks like reverse indexing should be handled. Why should the test fail if reverse indexing is handled?

[ryanyz10]

@epage It looks like this test:

assert_eq!(v!(""), filters!(Slice, v!("foobar"), v!(1), v!(0)));

is failing, which I think is expected behavior since the slice length is 0. The code in slice.rs line 68 has this:

if length < 1 {
    return invalid_argument("length", "Positive number expected").into_err();
}

which is the error I'm getting when I run the tests without #[should_panic].

[epage]

Weird, I wonder why I originally thought this was about negative numbers.

At least for the parts of liquid-rust I wrote, I wrote what I thought was the behavior. I went back and ported integration tests over from the Ruby implementation which resulted in these issues being created. The conformance tests take precedence over anything that looks like intended behavior, even other tests.

Of course I could have made a mistake in porting the tests. You can look at the file's history to when I checked in the ruby implementation or you can go look at the current Ruby version

Again, thanks for looking into this! You can also always reach me over on gitter with questions.

[ryanyz10]

From the Ruby conformance tests, it looks like a slice of length 0 should just return an empty string/array. I changed the condition above to length < 0 and the tests are passing now.