[vim/vim] XXD: color-support feature (PR #12131)
Aapo Rantalainen
Adds colors in xxd. Coloring the output might help some hexdumping tasks (visualizing).
The hex-value and value are both colored with the same color depending on the hex-value, e.g.:
- 0x00 = white
- 0xff = blue
- printable = green
- non-printable = red
- tabs and linebreaks = yellow
- (should be more?)
Each character needs 11 more bytes to contain color. (Same color in a row could contain only one overhead but the logic how xxd creates colums must be then changed.) Size of colored output is increased by factor of ~6. Also grepping the output will break when colors is used.
Flag for color is "-R", because less uses "-R".
Color uses parameters auto,always,never same as less and grep (among others).
E.g.
xxd -R always $FILE | less -R
Issues:
- _ _ MVS _ _ not tested
- not all col/group -combinations in little-endian (-e) works (check #12097)
Basic
ebcdic
Binary (bits are not colored)
Little-endian (Decimal offsets, Uppercase)
Test-script
Running 7632 combinations of flags run two times: without and with color (total of 15264 rounds). Color is then dropped with ansifilter (http://andre-simon.de/doku/ansifilter/en/ansifilter.php) and compared against known-good version of xxd.
When -c and -g are limited to the power-of-two then also all little-endian tests passed.
#!/bin/bash DATA=asciiA.txt XXD=./xxd_ff226d49fed2d8fc668084324c7b0f00117c5e74 function test_color_no() { echo -n ./xxd "$1" -R never " " str1=$( $XXD $1 $DATA | md5sum) color_no=$(./xxd -R never $1 $DATA | md5sum) if [ "$str1" == "$color_no" ]; then echo -n $(echo $str1 | cut -d" " -f1) " OK" else echo -n FAILED $str1 and $color_no fi echo } function test_color_yes() { echo -n ./xxd "$1" -R always " " str1=$( $XXD $1 $DATA | md5sum) color_yes=$(./xxd -R always $1 $DATA | ansifilter | md5sum) if [ "$str1" == "$color_yes" ]; then echo -n $(echo $str1 | cut -d" " -f1) " OK" else echo -n FAILED $str1 and $color_yes fi echo } function not_power_of_two () { declare -i n=$1 (( n > 0 && (n & (n - 1)) != 0 )) } for a in "" "-a" do for b in "" "-e" "-b" do for d in "" "-d" do for E in "" "-E" do for u in "" "-u" do for c in {1..21} do for (( g=1; g<=c; g++ )) do if [ "$b" == "-e" ] && not_power_of_two "$g" then : elif [ "$b" == "-e" ] && not_power_of_two "$c" then : else test_color_no "$a $b $d $E $u -c $c -g $g" test_color_yes "$a $b $d $E $u -c $c -g $g" fi done done done done done done done exit 0
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/12131
Commit Summary
- 2bbb3b9 XXD: added color-support
File Changes
(1 file)
- M src/xxd/xxd.c (259)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
Hi,
thanks for your contribution. I really like to include it. However, I have two questions:
- can you please add a test in test_xxd.vim
- can you please adjust the manpage
Thanks!
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aapo Rantalainen
@aapo pushed 3 commits.
- c4e87b0 Added test for xxd color
- 38e709c FIX: 'for' loop initial declarations are only allowed in C99 or C11 mode (xxd)
- 63bf2c3 xxd: -R (color) to manpage
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
codecov[bot]
Codecov Report
Merging #12131 (63bf2c3) into master (4df0772) will increase coverage by
1.38%.
Report is 476 commits behind head on master.
The diff coverage isn/a.
@@ Coverage Diff @@ ## master #12131 +/- ## ========================================== + Coverage 81.26% 82.65% +1.38% ========================================== Files 164 150 -14 Lines 194039 181473 -12566 Branches 43812 40795 -3017 ========================================== - Hits 157695 149995 -7700 + Misses 23622 18500 -5122 - Partials 12722 12978 +256
| Flag | Coverage Δ | |
|---|---|---|
| huge-clang-none | 82.65% <ø> (+<0.01%) |
⬆️ |
| huge-gcc-none | ? |
|
| huge-gcc-unittests | ? |
|
| linux | 82.65% <ø> (+0.37%) |
⬆️ |
| mingw-x64-HUGE | ? |
|
| windows | ? |
Flags with carried forward coverage won't be shown. Click here to find out more.
see 157 files with indirect coverage changes
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
some build error on Windows:
xxd.c(520): error C2065: 'STDOUT_FILENO': undeclared identifier
NMAKE : fatal error U1077: 'cl /nologo -DWIN32 xxd.c -link -subsystem:console' : return code '0x2'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.35.32215\bin\HostX64\x64\nmake.exe" /NOLOGO -f Make_mvc.mak ' : return code '0x2'
Stop.
Error: Process completed with exit code 2.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
LemonBoy
@LemonBoy commented on this pull request.
> @@ -415,4 +415,132 @@ func Test_xxd_little_endian_with_cols() bwipe! endfunc +func Test_xxd_color()
The test should use vim's screendump features.
In src/xxd/xxd.c:
> @@ -498,6 +517,11 @@ main(int argc, char *argv[]) char *pp; char *varname = NULL; int addrlen = 9; + int color=0;
Missing space around =, ditto for several other places in the newly-added code.
In src/xxd/xxd.c:
> @@ -498,6 +517,11 @@ main(int argc, char *argv[]) char *pp; char *varname = NULL; int addrlen = 9; + int color=0; + +#ifndef __MVS__
__MVS__ is checking if the host system is z/OS, which I believe is not what you meant.
In src/xxd/xxd.c:
> @@ -844,8 +888,61 @@ main(int argc, char *argv[])
c = addrlen + 1 + (grplen * x) / octspergrp;
if (hextype == HEX_NORMAL || hextype == HEX_LITTLEENDIAN)
{
+ if (color)
+ {
+ COLOR_PROLOGUE
+
+ if (ebcdic) /* EBCDIC */
This code is repeated a few lines below, consider introducing a lookup table for converting the EBCDIC/ASCII codepoint to color.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aapo Rantalainen
Aapo Rantalainen
@aapo commented on this pull request.
In src/xxd/xxd.c:
> @@ -844,8 +888,61 @@ main(int argc, char *argv[])
c = addrlen + 1 + (grplen * x) / octspergrp;
if (hextype == HEX_NORMAL || hextype == HEX_LITTLEENDIAN)
{
+ if (color)
+ {
+ COLOR_PROLOGUE
+
+ if (ebcdic) /* EBCDIC */
I reimplemented it with function.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aapo Rantalainen
@aapo commented on this pull request.
> @@ -415,4 +415,132 @@ func Test_xxd_little_endian_with_cols() bwipe! endfunc +func Test_xxd_color()
I don't know anything about vim's screendump features.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
LemonBoy
@LemonBoy commented on this pull request.
In src/xxd/xxd.c:
> @@ -482,6 +501,51 @@ static unsigned char etoa64[] =
0070,0071,0372,0373,0374,0375,0376,0377
};
+ static char
+begin_coloring_char(char *l,int *c,int e,int ebcdic) {
The spacing is still wonky (e.g. no space after commas).
The function's return type is char but it doesn't return anything. Perhaps make it return int so that the index c can be taken by value.
In src/xxd/xxd.c:
> @@ -482,6 +501,51 @@ static unsigned char etoa64[] =
0070,0071,0372,0373,0374,0375,0376,0377
};
+ static char
+begin_coloring_char(char *l,int *c,int e,int ebcdic) {
+ if (ebcdic) /* EBCDIC */
This comment made me laugh out loud, can be safely removed :D
In src/xxd/xxd.c:
> + if (hextype == HEX_BITS) + c += addrlen + 3 + p*12; + else + c = addrlen + 3 + (grplen * cols - 1)/octspergrp + p*12; + + if (hextype == HEX_LITTLEENDIAN) + c += 1; + + COLOR_PROLOGUE + begin_coloring_char(l,&c,e,ebcdic); + + #ifdef __MVS__ + if (e >= 64) l[c++] = e; + else l[c++] = '.'; + #else +
Extra line.
In src/xxd/xxd.c:
> @@ -498,6 +562,12 @@ main(int argc, char *argv[]) char *pp; char *varname = NULL; int addrlen = 9; + int color = 0; + +
Extra line.
In src/xxd/xxd.c:
> + l[c++] = '\n';
+ l[c] = '\0';
+ if (color)
+ {
+ c++;
+
+ int x = p;
+ if (hextype == HEX_LITTLEENDIAN)
+ {
+ int fill = octspergrp - (p % octspergrp);
+ if (fill == octspergrp) fill = 0;
+
+ c = addrlen + 1 + (grplen * (x - (octspergrp-fill))) / octspergrp;
+
+ int i;
+ for (i = 0;i < fill;i++)
Missing spaces after ;, you can declare i in the for loop.
In src/xxd/xxd.c:
> + COLOR_EPILOGUE
+ x++;
+ p++;
+ }
+ }
+
+ if (hextype != HEX_BITS)
+ {
+ c = addrlen + 1 + (grplen * x) / octspergrp;
+ c += cols - p;
+ c += (cols - p) / octspergrp;
+
+ int i;
+ for (i = cols - p;i > 0;i--)
+ {
+ COLOR_PROLOGUE
You can emit the prologue and epilogue only once and generate the n whitespaces.
Also, why is the space printed in red?
> @@ -415,4 +415,132 @@ func Test_xxd_little_endian_with_cols() bwipe! endfunc +func Test_xxd_color()
:h terminal-screendump
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aapo Rantalainen
@aapo commented on this pull request.
> @@ -415,4 +415,132 @@ func Test_xxd_little_endian_with_cols() bwipe! endfunc +func Test_xxd_color()
I have never used vim.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aapo Rantalainen
@aapo commented on this pull request.
In src/xxd/xxd.c:
> + COLOR_EPILOGUE
+ x++;
+ p++;
+ }
+ }
+
+ if (hextype != HEX_BITS)
+ {
+ c = addrlen + 1 + (grplen * x) / octspergrp;
+ c += cols - p;
+ c += (cols - p) / octspergrp;
+
+ int i;
+ for (i = cols - p;i > 0;i--)
+ {
+ COLOR_PROLOGUE
Color is not important but this is the easiest way to calculate/print the place when using -e
Without invisible (red) white spaces:
echo -n A | ./xxd -e -R never
00000000: 41 A
echo -n A | ./xxd -e -R always
00000000: 41 A
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aapo Rantalainen
> + l[c++] = '\n';
+ l[c] = '\0';
+ if (color)
+ {
+ c++;
+
+ int x = p;
+ if (hextype == HEX_LITTLEENDIAN)
+ {
+ int fill = octspergrp - (p % octspergrp);
+ if (fill == octspergrp) fill = 0;
+
+ c = addrlen + 1 + (grplen * (x - (octspergrp-fill))) / octspergrp;
+
+ int i;
+ for (i = 0;i < fill;i++)
Declaring variables inside a for loop wasn't valid C until C99.
Currently xxd can be compiled:
cc --ansi -DUNIX -o xxd xxd.c
I think this is not the reason to break that.
And after writing that I just figured I just broke --pedantic.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aapo Rantalainen
Aapo Rantalainen
> @@ -482,6 +501,51 @@ static unsigned char etoa64[] =
0070,0071,0372,0373,0374,0375,0376,0377
};
+ static char
+begin_coloring_char(char *l,int *c,int e,int ebcdic) {
I planned to use return value for something but then figured that passing pointer would be cleaner. (And then forgot type of return value.)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Aapo Rantalainen
@aapo pushed 2 commits.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
Thanks, I have made a few more changes (mainly to fix style and add tests) and I am about to merge the following:
https://github.com/vim/vim/compare/master...chrisbra:vim:gh-12131?expand=1
Any comments before I merge?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.