jsonnet icon indicating copy to clipboard operation
jsonnet copied to clipboard

feat: allow negative index/end on std.slice

Open Duologic opened this issue 1 year ago • 5 comments

I found myself implementing this elsewhere and figured it'd be easy to support in stdlib.

I ran make test, it succeeded for my changes but had unrelated failures, we might want to fix that in a separate PR.

Duologic avatar Jun 01 '23 19:06 Duologic

@sbarzowski Will this "just work" on go-jsonnet?

I think I was originally opinionated about not supporting this as I wanted the language to be simple... But in retrospect it seems more confusing to support python slicing only "partially" so this seems a good idea.

sparkprime avatar Jun 13 '23 20:06 sparkprime

Will this "just work" on go-jsonnet?

Yes. I think we just desugar to std.slice. That said ideally we would add a builtin (but that's neither easier nor harder with this change in).

BTW this change definitely requires an update to documentation.

sbarzowski avatar Jun 13 '23 22:06 sbarzowski

It seems like this was almost ready to go back in June, with only a request for documentation updates as a possible blocker. @Duologic would you like to contribute some doc changes for this, or should I work on it?

@sparkprime I'm not sure if there is any coordination needed with other jsonnet implementations to get this out? I assume we merge it here first, then go-jsonnet can pick it up (probably with the next release?) and other implementations (e.g., jrsonnet - which has a PR queued already, https://github.com/CertainLach/jrsonnet/pull/117) can update at their leisure.

johnbartholomew avatar Feb 10 '24 18:02 johnbartholomew

I just scanned the documentation and didn't find any mention that negative slices are not supported, am I missing something?

Do you want an example added maybe?

Duologic avatar Feb 12 '24 10:02 Duologic

Hmm... it's a good point, the tutorial says "Array slices like arr[10:20:2] are allowed, like in Python", so perhaps people have always assumed negative slices worked :-)

IMHO, yes, it would be good to add an example of a negative index in the std.slice documentation in the standard library reference.

johnbartholomew avatar Feb 12 '24 11:02 johnbartholomew

Rebased and merged. :-)

johnbartholomew avatar Mar 02 '24 18:03 johnbartholomew