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
Issue2936 #6799
base: main
Are you sure you want to change the base?
Issue2936 #6799
Conversation
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.
Thanks for the PR! 😃 🍰
@@ -67,6 +68,26 @@ def format_keyvals( | |||
if indent > 2: | |||
indent -= 2 # We use dividechars=2 below, which already adds two empty spaces | |||
|
|||
cols, _ = urwid.raw_display.Screen().get_cols_rows() |
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 haven't tested this, but this looks like it will fall apart when using layouts that don't span the entire width. Can we pass in the available space instead?
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.
Hi @mhils ,
thanks for the review and the awesome work you people do here!
I'm not sure I'm following you: my (admittedly empirical) tests proved the fix to
work on layouts smaller than full screen (see attached screenshot - header values are not wrapped even if the terminal size is half that of the screen, instead the header name is wrapped at 30 chars). Are there different scenarios you're thinking of?
Furthermore, I investigated a bit more and saw that mitmproxy already uses the same way to retrieve screen columns elsewhere, for instance in mitmproxy/tools/console/statusbar.py (note that self.master.ui
inherits from urwid.raw_display.Screen
as it can be seen in mitmproxy/tools/console/window.py).
Anyhow I am by no means an expert on urwid
, so I'll be glad to refine the PR if you still think this approach is broken.
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.
Are there different scenarios you're thinking of?
\You can press -
to toggle between views. If you enable the two column view, urwid.raw_display.Screen().get_cols_rows()
will give you twice the space available:
The statusbar "gets away" with this by always being full width.
After looking into this for a bit, the correct fix is unfortunately more complex. Urwid widgets famously have no widths, so we'd probably need to implement a custom widget here or something like that. :/ I haven't looked super close, but it looks a bit ugly.
Description
My attempt at increasing header name line-wrap length according to unused console columns. Specifically:
Fixes #2936 .
Checklist