google / jsonnet

Jsonnet - The data templating language
http://jsonnet.org
Apache License 2.0
6.98k stars 440 forks source link

feat: allow negative index/end on std.slice #1093

Closed Duologic closed 8 months ago

Duologic commented 1 year ago

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.

sparkprime commented 1 year ago

@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.

sbarzowski commented 1 year ago

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.

johnbartholomew commented 8 months ago

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.

Duologic commented 8 months ago

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?

johnbartholomew commented 8 months ago

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 commented 8 months ago

Rebased and merged. :-)