Add JQ_COLORS env var for color config (fix #1252) #1397
Conversation
|
This addresses #1252. |
src/jv_print.c
Outdated
| if (c == NULL) | ||
| return 1; | ||
| colors = def_colors; | ||
| memset(color_bufs, 0, sizeof(colors)); |
There was a problem hiding this comment.
Should this be sizeof(color_bufs)?
src/jv_print.c
Outdated
| for (i = 0; i < sizeof(def_colors) / sizeof(def_colors[0]) && *c != '\0'; i++, c = e) { | ||
| if ((e = strchr(c, ':')) == NULL) | ||
| e = c + strlen(c); | ||
| if ((size_t)(e - c) > sizeof(color_bufs[i]) - 1) |
There was a problem hiding this comment.
This should be sizeof(color_bufs[i])-1-3 (one for the NUL, and 3 for the \033[m)
There was a problem hiding this comment.
Thanks! (Though I attempted to test this.)
src/jv_print.c
Outdated
| return 0; | ||
| color_bufs[i][0] = ESC[0]; | ||
| color_bufs[i][1] = '['; | ||
| (void) strncpy(&color_bufs[i][2], c, e - c); |
There was a problem hiding this comment.
This can overflow without the fix above (admittedly only by two bytes, but that's an overflow)
There was a problem hiding this comment.
It certainly doesn't overflow now (there's a test).
src/jv_print.c
Outdated
| (void) strncpy(&color_bufs[i][2], c, e - c); | ||
| if (strspn(&color_bufs[i][2], "0123456789;") < strlen(&color_bufs[i][2])) | ||
| return 0; | ||
| color_bufs[i][2 + e - c] = 'm'; |
There was a problem hiding this comment.
Same. 2+15 = 17, which is 2 past the end of your 16-byte buffer.
There was a problem hiding this comment.
Also, you're not guaranteed to write a NUL byte into color_bufs[i][3 + e - c]. Let's explicitly do that so we can't run off the end later.
There was a problem hiding this comment.
That's what the memset() is for.
src/jv_print.c
Outdated
| memset(color_bufs, 0, sizeof(colors)); | ||
| for (i = 0; i < sizeof(def_colors) / sizeof(def_colors[0]); i++) | ||
| color_bufps[i] = def_colors[i]; | ||
| for (i = 0; i < sizeof(def_colors) / sizeof(def_colors[0]) && *c != '\0'; i++, c = e) { |
There was a problem hiding this comment.
Here, are we wanting to iterate across all 8 possible color_bufs or only the 7 we default with?
src/jv_print.c
Outdated
| @@ -27,11 +27,46 @@ | |||
| static const jv_kind color_kinds[] = | |||
| {JV_KIND_NULL, JV_KIND_FALSE, JV_KIND_TRUE, JV_KIND_NUMBER, | |||
| JV_KIND_STRING, JV_KIND_ARRAY, JV_KIND_OBJECT}; | |||
| static const char* const colors[] = | |||
| static char color_bufs[8][16]; | |||
There was a problem hiding this comment.
Why 8 if there are only 7 def_colors?
There was a problem hiding this comment.
Uh, no reason. I'll change it.
tests/shtest
Outdated
| cmp $d/color $d/expect | ||
| JQ_COLORS='1;31234567890123456789:0;31:0;32:0;33:0;34:1;35:1;36' \ | ||
| $JQ -Ccn '[{"a":true,"b":false},123,null]' > $d/color | ||
| cmp $d/color $d/expect |
There was a problem hiding this comment.
Let's also add a test where we expect failure with a strings like 12345678901234:12345678901234:12345678901234:12345678901234:12345678901234:12345678901234:12345678901234 (for buffer overrun)
and 1234567890123:1234567890123:1234567890123:1234567890123:1234567890123:1234567890123:1234567890123 (for lack of trailing NUL)
|
Fixes pushed. |
|
Thanks! |
This commit adds an env var,
JQ_COLORS, that allows the user to specify an alternative color scheme.You can't see it here, but this produces something of a rainbow: