Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Inconsistency with error keyword output between sjsonnet and go-jsonnet #77

Closed
Qinusty opened this issue Jun 18, 2020 · 1 comment · Fixed by #174
Closed

Inconsistency with error keyword output between sjsonnet and go-jsonnet #77

Qinusty opened this issue Jun 18, 2020 · 1 comment · Fixed by #174

Comments

@Qinusty
Copy link
Contributor

Qinusty commented Jun 18, 2020 •
edited

With go-jsonnet, the error keyword triggers a failure with an error message and some form of trace.

sjsonnet however, simply errors with status code 1 and nothing sent to stdout or stderr.

Reproducer

jsonnet -e "error 'Foo.'"
java -jar ~/Downloads/sjsonnet.jar -e "error 'Foo.'"

This should be tracked by #73
@rhanqtl
Copy link

rhanqtl commented Aug 26, 2020 •
edited

The reason is that sjsonnet 0.2.6 does not support the -e or --exec option as go-jsonnet does. Print the help or read the source code (sjsonnet/src-jvm/sjsonnet/Cli.scala) to check this.

@lihaoyi-databricks lihaoyi-databricks mentioned this issue Jun 5, 2023
Merged
lihaoyi-databricks added a commit that referenced this issue Jun 6, 2023
@lihaoyi-databricks
Fixes for main method (#174)
c729e28 
Bug fixes and unit tests to validate behavior of various command line
flags and their combinations: `--exec`, `--yaml-out`, `--yaml-stream`,
`--multi`, `--output-file`. I also propagated the `std: Std` argument
all the way to `sjsonnet.SjsonnetMain.main0`, so it can easily be passed
in by people who use the CLI-arg-based programmatic interface without
having to drop down into `sjsonnet.Interpreter`, e.g. in Databricks'
`JsonnetWorker`

Fixes #112, and fixes
#77, and adds tests for
#60 (which was already
working before, but without test coverage)

The logic inside `SjsonnetMain.scala` is still pretty messy, but at
least it has some rudimentary unit tests now.

Another thing to note is that trailing newline handling isn't great;
depending on what code path you go through, you may get zero, one, or
two trailing newlines. That should be benign, since trailing newlines
are not semantically meaningful in either JSON or YAML, so for now I
just left them in place. At least now we have tests to surface some of
the weirdness

I had to move `MainTests.scala` from `src-jvm-native/` to `src-jvm/`,
due to weird crashes in the Scala Native test-running infrastructure.
Given the experimental nature of Scala-Native, losing a bit of coverage
is probably OK for now, and also it's unlikely that this will cause
anything to break given the same code receives test coverage on the JVM.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes for main method databricks/sjsonnet
2 participants
@Qinusty @rhanqtl