search

radareorg / radare2

UNIX-like reverse engineering framework and command-line toolset
https://www.radare.org/
GNU Lesser General Public License v3.0
20.72k stars 3.01k forks source link

Only 4 bits per RGB channel can be used via the 'ec' command family #3556

Closed sghctoma closed 9 years ago

sghctoma commented 9 years ago

Hi,

Yesterday I was trying to create a Monokai-ish color theme for radare2, and I got a bit confused. The fact that we can't use the whole 24bit color space despite the fact that it is indeed supported by radare2 was what confused me. We can set colors using commands like this: "ec mov rgb:abc". This means, that we can only control 4 bits of each component (r, g, b) instead of 8.

(It is also not clear what color "rgb:abc" actually is. First I thought it means

aabbcc (since this is how the 3-digit notation is used when referring to websafe

colors), but looking at the source code quickly became clear that it means

a0b0c0.)

My question is this: is this limitation there for a reason? I've patched libr/cons/pal.c (in https://github.com/sghctoma/radare2/tree/true-truecolor) so that "ec" accepts 6 digits (e.g. rgb:272822), and - to not break existing color schemes - also the 3 digits color notations, and it seems to work well.

Also, it is pointless for 'ecs' to show the current color chart when only 256 colors are used - the displayed colors won't match the displayed RGB values anyways. For example, consider the following commands:

[0x00000000]> ec prompt rgb:6c6 # rgb:6c6 is a color from the displayed chart
[0x00000000]> ec*~prompt
ec prompt rgb:8d8
[0x00000000]>

Because of this, I've also patched the command 'ecs', so that it displays:

What do you think about this?

Regards, Toma

P.S. I do realize that this is not the most important thing, but it nagged me, so I just had to dig deeper.

radare commented 9 years ago

can you submit a pullreq for review?

the change makes sense to me

On 22 Oct 2015, at 17:26, sghctoma notifications@github.com wrote:

Hi,

Yesterday I was trying to create a Monokai-ish color theme for radare2, and I got a bit confused. The fact that we can't use the whole 24bit color space despite the fact that it is indeed supported by radare2 was what confused me. We can set colors using commands like this: "ec mov rgb:abc". This means, that we can only control 4 bits of each component (r, g, b) instead of 8.

(It is also not clear what color "rgb:abc" actually is. First I thought it means

aabbcc (since this is how the 3-digit notation is used when referring to websafe

colors), but looking at the source code quickly became clear that it means

a0b0c0.)

My question is this: is this limitation there for a reason? I've patched libr/cons/pal.c (in https://github.com/sghctoma/radare2/tree/true-truecolor https://github.com/sghctoma/radare2/tree/true-truecolor) so that "ec" accepts 6 digits (e.g. rgb:272822), and - to not break existing color schemes - also the 3 digits color notations, and it seems to work well.

Also, it is pointless for 'ecs' to show the current color chart when only 256 colors are used - the displayed colors won't match the displayed RGB values anyways. For example, consider the following commands:

[0x00000000]> ec prompt rgb:6c6 # rgb:6c6 is a color from the displayed chart [0x00000000]> ec*~prompt ec prompt rgb:8d8 [0x00000000]> Because of this, I've also patched the command 'ecs', so that it displays:

only the basic ANSI colors when scr.rgbcolor is false. the basic colors + the XTerm grayscale + the XTerm colors from 16 to 231 when scr.rgbcolor is true, but scr.truecolor is false. the basic colors + the XTerm grayscale + the color chart that is currently used when both scr.rgbcolor and scr.truecolor are true. What do you think about this?

Regards, Toma

P.S. I do realize that this is not the most important thing, but it nagged me, so I just had to dig deeper.

— Reply to this email directly or view it on GitHub https://github.com/radare/radare2/issues/3556.

sghctoma commented 9 years ago

Sure, I will, but first a question: the coding style in these parts of the code does not really match your guidelines found in CONTRIBUTING.md (e.g. no spaces around operators). Should I format the code according to the guidelines, or use the style used in the original code? Right now it's a bit of a mix - new/modified code is formatted according to the guidelines, but I did not reformat anything else.

radare commented 9 years ago

This code was written before that update on the codingstyle. I plan to update the indentation of yhe whole project in an automated way when clangformat is able to do this.

Until that day, we are doing progressive fixes to enhace the indentation of the whole codebase.

If you do so i would recommend you to do it in two commits. so your changes are easier to review

On 22 Oct 2015, at 19:04, sghctoma notifications@github.com wrote:

Sure, I will, but first a question: the coding style in these parts of the code does not really match your guidelines found in CONTRIBUTING.md (e.g. no spaces around operators). Should I format the code according to the guidelines, or use the style used in the original code? Right now it's a bit of a mix - new/modified code is formatted according to the guidelines, but I did not reformat anything else.

— Reply to this email directly or view it on GitHub.