-
-
Notifications
You must be signed in to change notification settings - Fork 719
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
Variadic gef print #789
Variadic gef print #789
Conversation
71775b1
to
ad50f72
Compare
As mentioned briefly on Discord, wouldn't be simpler to just do this gef_print(f"\tParent PID {RIGHT_ARROW} {state['Pid']}"
f"\tCommand line {RIGHT_ARROW} '{cmdline}'") |
there wouldn't be newlines between each line in that case, they'd be concatenated to one line. Look at my comments above. |
e978144
to
fa28aa2
Compare
ad50f72
to
52935ca
Compare
Yes, this probably should have been a 'discussion' (I am not very familiar with them). We can close, but I want to keep the diff & have @theguy147 & @daniellimws weigh in oh what they think about this approach. |
For me, slightly confusing at the start but should get used to it after a while. I think it could be worth the change if |
I'm for the "variadic" printing but I don't like how the How about something like this?: def gef_print(*args: str, sep: str = "\n", **kwargs: Any) -> Optional[int]:
"""Wrapper around `print()`, using string buffering feature."""
highlighted = list(map(highlight_text, args))
if gef.ui.stream_buffer and not is_debug():
return gef.ui.stream_buffer.write(sep.join(highlighted))
print(*highlighted, sep=sep, **kwargs) EDIT: def gef_print(*args: str, sep: str = "\n", **kwargs: Any) -> None:
"""Wrapper around `print()`, using string buffering feature."""
highlighted = list(map(highlight_text, args))
if gef.ui.stream_buffer and not is_debug():
gef.ui.stream_buffer.write(sep.join(highlighted))
else:
print(*highlighted, sep=sep, **kwargs) We never even use the EDIT: |
Yeah, I was using Agreed on the rv of all those functions. They should be removed. I think that we should support variadic using |
That's why in my draft I set |
Yeah, but I am not sure if we want |
ok, I thought that was kind of the whole point about this PR. Just naming it differently but still changing the code to have this behavior doesn't make sense to me |
Well |
Yeah, I didn't question the existance of |
I'm saying that the variadic arguments should NOT have a newline between them, since that makes |
ah ok. so then I agree xD |
52935ca
to
b262d81
Compare
Redid it so it acts like |
043136b
to
e146d19
Compare
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.
let's make gef_print
great again
gef.py
Outdated
return gef.ui.stream_buffer.write(x + kwargs.get("end", "\n")) | ||
print(x, *args, **kwargs) | ||
gef.ui.stream_buffer.write( | ||
sep.join(highlight_text(a) for a in args) + end) |
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.
sep.join(highlight_text(a) for a in args)
is repeated, can be simplify imo
text = sep.join(highlight_text(a) for a in args)
if gef.ui.stream_buffer and not is_debug():
gef.ui.stream_buffer.write(text+ end)
return
print(text, end=end, **kwargs)
return
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.
yeah that's good.
Note that you can click and drag the +
for a comment over multiple lines, and that way you can give a multi line suggestion.
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.
how about this then?
text = [highlight_text(a) for a in args]
if gef.ui.stream_buffer and not is_debug():
gef.ui.stream_buffer.write(sep.join(text) + end)
else:
print(text, sep=sep, end=end, **kwargs)
that way we are closer to how pythons builtin print
works. (and IMO it would also be nice to start removing unnecessary trailing returns at the end of functions)
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.
yeah that's better actually, though we'd need to do print(*text...
otherwise the print would but []
around it.
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.
(And hugsy likes those returns because he likes how they are explicit)
Co-authored-by: hugsy <[email protected]>
Co-authored-by: hugsy <[email protected]>
Co-authored-by: hugsy <[email protected]>
Co-authored-by: hugsy <[email protected]>
Co-authored-by: hugsy <[email protected]>
gef_print
take variadic arguments likeprint
does.gef_print
not return anything likeprint
doeswarn
,info
, etc. not return anything sincegef_print
doesn't.Closes #788