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.

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

Support queries where outputs are not 1:1 with inputs #28

Merged
merged 2 commits into from
Apr 15, 2023

Conversation

charles-dyfis-net
Copy link
Contributor

Per #27. Implements a (slower) MultiOutputContainer that generates a JSON list of the query's results when given the active input, used when :multi true is set.

@charles-dyfis-net

This comment was marked as resolved.

@charles-dyfis-net charles-dyfis-net marked this pull request as ready for review April 14, 2023 15:37
@dainiusjocas
Copy link
Owner

It turns out that this PR is not a "feature request" it is actually a bug fix 🥇

My original implementation assumed that PathOutput#emit is called only once per call to JsonQuery#apply which is not correct. This is clear when debugging your examples in #27.

@charles-dyfis-net
Copy link
Contributor Author

Hmm; if that's the case it might call for changing the :multi false default, or at least documenting reliance on that default as deprecated and due to be changed in the next major release. I yield to your judgment on matters of balancing correctness and compatibility.

@dainiusjocas
Copy link
Owner

Hmm; if that's the case it might call for changing the :multi false default, or at least documenting reliance on that default as deprecated and due to be changed in the next major release. I yield to your judgment on matters of balancing correctness and compatibility.

For now, we definitely need to make :multi false the default. And then I'll add more test cases so that the default behaviour is compatible with jq.

@dainiusjocas
Copy link
Owner

The tricky bit is that JQ always returns a stream (0 or more) of json entities https://stackoverflow.com/a/69303619/1728133.

A helpful intro https://github.com/pkoppstein/jq/wiki/A-Stream-oriented-Introduction-to-jq

@charles-dyfis-net
Copy link
Contributor Author

charles-dyfis-net commented Apr 15, 2023

Indeed so. A more designed-to-purpose interface might return a seq, which would allow laziness as an implementation detail; as it is, though, returning an ArrayNode is returning a seqable, which this PR takes advantage of in the CLI implementation.

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