fix: align std.lines with official semantics#1079
Open
He-Pin wants to merge 1 commit into
Open
Conversation
Motivation: go-jsonnet issue databricks#885 exposed confusion around std.lines: official Jsonnet treats it as array-to-text, not string-to-array. sjsonnet already rejected string input, but the native implementation materialized arrays unnecessarily and lacked a focused regression for this edge. Modification: Rewrite std.lines to build the newline-terminated string directly from Val.Arr while preserving null-skipping behavior. Add compatibility coverage and a golden error case for string input. Result: std.lines keeps the official array-to-text behavior, rejects string input, and avoids an unnecessary ujson materialization pass. References: google/go-jsonnet#885
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
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:
Official Jsonnet documents
std.lines(arr)as concatenating an array of strings into a text file with a newline after each string. It is not a line-splitting function for string input.The native sjsonnet implementation should therefore preserve array-to-text semantics and reject non-array input. This also resolves the behavioral confusion discussed in google/go-jsonnet#885 without treating the upstream issue thread as the specification.
Modification:
std.linespath to iterate overVal.Arrdirectly with aStringBuilder.std.join-compatible handling fornullarray elements.Result:
std.lines(["a", "b"])follows the documented text-file construction behavior, including the trailing newline.std.lines("a\nb\n")remains a type error because the documented parameter is an array. The table below includes the old sjsonnet behavior frommasterbefore this PR.std.lines(["a", "b"])"a\nb\n""a\nb\n""a\nb\n""a\nb\n"std.lines(["a", null, "b"])"a\nb\n""a\nb\n""a\nb\n""a\nb\n"std.lines("a\nb\n")[std.lines] Wrong parameter type: expected Array, got stringThe observed external behavior on
masteralready matches the documented cases above; this PR keeps that behavior explicit with focused native-path tests and implementation cleanup.References:
std.linesdocumentation: https://jsonnet.org/ref/stdlib.html#std-lines