Skip to content

fix: --yaml-stream produces correct output matching C++ jsonnet/go-jsonnet#1026

Merged
stephenamar-db merged 1 commit into
databricks:masterfrom
He-Pin:fix/yaml-stream-format
Jun 24, 2026
Merged

fix: --yaml-stream produces correct output matching C++ jsonnet/go-jsonnet#1026
stephenamar-db merged 1 commit into
databricks:masterfrom
He-Pin:fix/yaml-stream-format

Conversation

@He-Pin

@He-Pin He-Pin commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR fixes --yaml-stream (-y) output to match C++ jsonnet and go-jsonnet stream behavior.

Before this PR, sjsonnet rendered the top-level array using YAML-style output without the JSON document stream markers expected by C++ jsonnet/go-jsonnet.

Behavior comparison

Local reference versions used:

  • C++ jsonnet v0.22.0
  • go-jsonnet v0.22.0
  • jrsonnet 0.5.0-pre99
Case C++ jsonnet go-jsonnet jrsonnet sjsonnet before sjsonnet after
[1, 2] with -y ---\n1\n---\n2\n...\n same as C++ same as C++ missing stream markers / YAML-style rendering same as C++/go
[] with -y empty output empty output \n...\n wrong stream rendering empty output
{a: 1} with -y error: top-level object should be array same as C++ error with different wording silently rendered normally error matching C++/go wording

The intentional compatibility target is C++ jsonnet/go-jsonnet. jrsonnet differs for empty arrays and error wording.

Changes

  • SjsonnetMainBase.scala: render YAML streams as JSON documents separated by --- and terminated by ..., with empty arrays producing no output
  • SjsonnetMainBase.scala: reject non-array top-level values in stream mode with the C++/go-jsonnet style error
  • MainTests.scala: update expected output and add non-array / empty-array regression coverage

Tests

rtk ./mill 'sjsonnet.jvm[3.3.7]'.test.testOnly sjsonnet.MainTests
rtk git diff --check upstream/master...HEAD

Both passed locally after rebasing onto upstream/master.

@stephenamar-db

Copy link
Copy Markdown
Collaborator

rebase

@He-Pin He-Pin force-pushed the fix/yaml-stream-format branch from 0ea50d5 to 8274626 Compare June 24, 2026 17:14
…onnet

The --yaml-stream (-y) CLI flag had four bugs:
1. Missing initial `---` document start marker
2. Missing `...` document end marker
3. Rendered each document as YAML instead of JSON
4. Did not error on non-array input (silently fell through to normal rendering)

Cross-implementation comparison for input `[{"a":1},{"b":2}]`:

| Feature              | C++ jsonnet    | go-jsonnet     | sjsonnet (before) | sjsonnet (after) |
|----------------------|----------------|----------------|-------------------|------------------|
| Initial `---`        | Yes            | Yes            | No                | Yes              |
| Trailing `...`       | Yes            | Yes            | No                | Yes              |
| Document format      | JSON           | JSON           | YAML              | JSON             |
| Non-array error      | Yes            | Yes            | No (silent)       | Yes              |
| Empty array output   | (empty)        | (empty)        | `\n`              | (empty)          |

Output is now byte-identical to C++ jsonnet for all tested cases.

Root cause: The YAML stream code in SjsonnetMainBase.scala used
`config.copy(yamlOut = true)` which forced YAML rendering instead of JSON,
had incorrect `---` prefix logic (only for scalars or non-first elements),
and fell through to `renderNormal` for non-array values instead of erroring.

Fix: Rewrite the YAML stream handler to:
- Write `---\n` before each document
- Render each document as JSON using the standard Renderer
- Write `...\n` after all documents
- Produce an error with the value's type name for non-array input
- Write directly to stdout stream to avoid double-newline from println
- Handle --output-file correctly for the YAML stream case
@He-Pin He-Pin force-pushed the fix/yaml-stream-format branch from 8274626 to 3053b17 Compare June 24, 2026 17:59
@stephenamar-db stephenamar-db merged commit fafd01d into databricks:master Jun 24, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants