wargio / r2dec-js

radare2 plugin - converts asm to pseudo-C code.
514 stars 50 forks source link

background coloring #183

Closed imyxh closed 5 years ago

imyxh commented 5 years ago

So I think I've figured out that when r2dec displays inline assembly output, it's colored with the foreground color of "usrcmt" as set by the radare2 theme.

With many themes, this looks just fine. But with some, like the built-in bold and ogray themes, usrcmt is meant to be dark on light:

ec usrcmt rgb:000 rgb:f72  # from ogray

Of course, on dark terminals this means that the text will be pretty much invisible, since the f72 background is dropped.

image

This could be fixed by just honoring the theme's choice of background color.

But tangentially, why does r2dec even use the usrcmt color for inline assembly, anyway? The usrcmt style is for user-defined comments, which are assumed to be fairly sparse in the disassembly view. It makes sense for themes to assign really obnoxious colors to usrcmt so that the few comments there are will stand out. Conversely, r2dec spits out a lot of assembly, and for many themes would look really ugly if the background colors of usrcmt were ported properly.

I think r2dec should really use another color key other than usrcmt for assembly code. Perhaps use "other"? I can move this to another issue if you like.

wargio commented 5 years ago

there are 2 ways to fix this: Fix 1 (which does not require me and you to touch the code): create a theme manually and define it via r2dec.theme, see default.json (it has to be in the same folder as default.json in your r2dec install path). Fix 2 (requires a patch): handle background color.

Besides those fixes, can you produce an issue via pddi of that x86 code so i can add the missing assembly instructions?

imyxh commented 5 years ago

Aha. Looks like the missing assembly instructions are because I'm using AT&T syntax. #184

I would've gladly defined my own theme except I couldn't actually find a setting for the colors of inline assembly in the default.json file.

wargio commented 5 years ago

it's the string color one.

imyxh commented 5 years ago

Ah, so "text". Works for me, thanks!