-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix for #2374 #2375
Fix for #2374 #2375
Conversation
LGTM |
This needs to be a command-line option. As @pkoppstein explained in #2374, this would be an incompatible change. |
You can probably use |
As @pkoppstein explained in #2374, this change breaks backwards compatibility. Closing for now, will revisit as an additional commandline if I get time. |
I don't think it matters that fixing this bug "breaks backward compatibility". I think this fix should be merged. The fix looks good, but there a merge conflict in shtest. |
Actually, this patch does not fix the issue fully correctly. If you use $ printf foo\\n > foo; printf bar > bar; printf baz1\\nbaz2\\n > baz; printf xyz > xyz; printf hello > hello
$ jq -sR . foo bar baz xyz hello
"foo\nbar"
"baz1"
"baz2"
"xyz"
"hello" |
And this problem also occurs for files in non-raw mode: $ jq . <(printf 4) <(echo 5)
45 |
Let's fix this in a separate PR. |
See #2374 for explanation of the issue.
I've poked around the code and found this possible fix. It's not very neat (checking
state->current_input
and modifyingstate->current_line
outside ofjq_util_input_read_more
), but it works and doesn't break any other tests. Please feel free to propose a neater fix if you can see one.I've also added a regression test. This uses a bunch of functions, so perhaps shouldn't be in
shtest
? But I don't seeinput_line_number
tested anywhere else.