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

Better dumping #1993

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Better dumping #1993

wants to merge 6 commits into from

Conversation

bb010g
Copy link

@bb010g bb010g commented Oct 22, 2019

A fair amount of refactoring went into this PR to make each step clear and prevent issues like those in #1789, where escaping wasn't correctly handled. I'll summarize the non-bugfix commits, and the commit messages for the fixes follow. jvp_dump_string changed from consuming a boolean-use int ascii_only to enum jv_print_flags int flags. This makes jvp_dump_string more capable of responding to input formats, reducing pressure on consumers to get it right. jvp_dump_char is factored out from jvp_dump_string, making future increases in complexity to both clearer. JV_PRINT_RAW and JV_PRINT_RAWCOLOR flags are introduced to enum jv_print_flags, consolidating prints in main.c's process to jv_dump(result, dumpopts). The JV_PRINT_RAW flag is not passed down to recursive jv_dump* calls for arrays or objects. JV_PRINT_RAWCOLOR is unset by default and has no API currently, but is there if colored, raw output is desired in the future. JV_PRINT_RAWCOLOR avoids problems with colored output by noticing when a type would be raw printed and disabling JV_PRINT_COLOR before that's a problem.

Fix stderr/0 builtin to match documentation

It's supposed to print raw, compact, and with no decoration or trailing newline. Previously, it would print JSON formatted, compact, with no trailing newline and no easy way to obtain one, and ignoring the --ascii-output / -a flag.

f_stderr, instead of calling jv_dumpf(jv_copy(input), stderr, 0), now has a stderr_cb setup similar to f_debug that it uses identically. The only functional difference is main.c's stderr_cb:

static void stderr_cb(void *data, jv input) {
  int dumpopts = *(int *)data;
  jv_dumpf(input, stderr, (dumpopts & ~JV_PRINT_PRETTY) | JV_PRINT_RAW);
  // debug_cb prints a newline here.
}

Bring --raw-output and --ascii-output to agreement

Escapes are still printed whenever characters outside the ASCII plane are encountered. To avoid ambiguity, backslash is the only ASCII character escaped (as \\).

Fixes #1788, properly this time. Closes #1789.

jvp_dump_char gains a boolean-use int raw parameter that is used to always print ASCII characters (save backslash) and turn on escapes for everything else. jvp_dump_string learns to handle raw strings in the non-(raw && !ascii_only) path. Due to the earlier changes, every other use of raw strings is immediately corrected.


This change is Reviewable

@coveralls
Copy link

coveralls commented Oct 26, 2019

Coverage Status

Coverage decreased (-0.1%) to 84.005% when pulling 5f0f353 on bb010g:better-dump into a17dd32 on stedolan:master.

@bb010g
Copy link
Author

bb010g commented Oct 26, 2019

Rebased and force pushed to run Travis after the fixes.

@bb010g
Copy link
Author

bb010g commented Jan 15, 2020

Is there anything I can do to help this get reviewed & in-tree?

@nicowilliams
Copy link
Contributor

I'll take a look as soon as I can, which might not be till next week :(

@bb010g
Copy link
Author

bb010g commented Jun 10, 2020

bump again

@bb010g
Copy link
Author

bb010g commented Sep 9, 2020

Rebump.

Also commented to make the implementation clearer.
Behavior is the same, but this form is easier to understand and make
robust.
It's supposed to print raw, compact, and with no decoration or trailing
newline. Previously, it would print JSON formatted, compact, with no
trailing newline and no easy way to obtain one, and ignoring the
--ascii-output / -a flag.
Escapes are still printed whenever characters
outside the ASCII plane are encountered. To avoid
ambiguity, backslash is the only ASCII character
escaped (as `\\`).

Fixes jqlang#1788, properly this time. Closes jqlang#1789.
@bb010g
Copy link
Author

bb010g commented Sep 20, 2020

Rebased with manual review over new commits (24564b2 got close to this patch's lines), and still passes tests.

(AppVeyor is experiencing pacman mingw32.db issues, unrelated to this patchset.)

@vergenzt
Copy link

vergenzt commented Nov 9, 2021

This PR addresses #2063

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jq-1.6 - raw output doesn't work when -a is specified
5 participants