-
Notifications
You must be signed in to change notification settings - Fork 847
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
feat: Add convenience functions for printing bold colors #2157
base: dev
Are you sure you want to change the base?
Conversation
What if colors took optional arguments like |
Ya, that would work too. Could then add underline, italics, etc flags that way too. I'm fine with whatever you all think is best. |
One caveat to using flags is, for things like memory coloring, is that pwndbg uses a table like this: c = ColorConfig(
"memory",
[
ColorParamSpec("stack", "yellow", "color for stack memory"),
ColorParamSpec("heap", "blue", "color for heap memory"),
ColorParamSpec("code", "red", "color for executable memory"),
ColorParamSpec("data", "purple", "color for all other writable memory"),
ColorParamSpec("rodata", "normal", "color for all read only memory"),
ColorParamSpec("rwx", "underline", "color added to all RWX memory"),
ColorParamSpec("gap", "bold_white", "color added for unmapped regions of memory"),
],
) I've added an example "gap" entry to demonstrate someone might want to differentiate the line by using a bold color. Your suggestion wouldn't work there due to the way the generated functions work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with those changes: I am a bit worried, that we introduce more and more redundant func calls where each of them also performs a global fetch but w/e: some of these are probably cached elsewhere.
In the future, we should probably just remove all the BOLD = ...
constants if no other code uses it (and it shouldn't) as well as calls to bold(black(x))
and just inline it in within the functions. Kinda a microoptimization, but some of the coloring is called many many times and it may be beneficial on the long run.
@gsingh93 if we add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets add the bold argument. An additional if statement is better than a number of redundant function calls.
I'll switch it around |
eab8b45
to
b913546
Compare
Switched to use bold argument. I've also switched the other muslheap thing that I'm going to send a PR for soon to use this and seems to work fine. |
Previously if you wanted to use bold colors people would use a lambda or manually use 2 functions, for example:
These would let you just use
pwndbg.color.bold_green()
in the first case orfrom pwndbg.color import bold_green
for the second case.A tool I'm porting uses bolded colors for lots of stuff, which I'm keeping the same for now, so am using a bunch of these new functions.