rndusr / stig

TUI and CLI for the BitTorrent client Transmission
GNU General Public License v3.0
554 stars 24 forks source link

Use urwid>=2.5.0 Scrollable and ScrollBar widgets (Fixes #244) #247

Closed rsekman closed 3 months ago

rsekman commented 3 months ago

The Scrollable and ScrollBar widgets implemented for stig were merged into urwid in 2.5.0. This PR updates stig to use the now native urwid widgets. This makes stig compatible with urwid>=2.5.0. However, this PR depends on https://github.com/urwid/urwid/pull/879 being merged and released. This is because stig tries to scroll widgets inside AttrMap in several places. After that bug is fixed, we'll need to bump the urwid requirement to whatever release it ends up in (likely 2.6.12), and then stig's version.

rndusr commented 3 months ago

Looks like this wasn't fixed yet. urwid 2.6.11 still gives me the "TypeError: Not a scrollable widget" error.

If it is no trouble for you, could you ping me when the fix in urwid is released?

rsekman commented 3 months ago

@rndusr the needed code has now been merged and released in urwid 2.6.12 https://github.com/urwid/urwid/releases/tag/2.6.12

rndusr commented 3 months ago

Great, thank you very much!

I'll make a new release tomorrow if this keeps running smoothly.

If you are bored, I'm getting this warning:

[...]/python3.11/site-packages/urwid/widget/grid_flow.py:81: GridFlowWarning: Size is smaller than cell width (-1 < 20)
  super().__init__(self.generate_display_widget((self._cache_maxcol,)))

It looks like it's caused by TabBar, which is the only widget using GridFlow. (Aside from TorrentDetailsWidget, which shouldn't be created before the TUI is running.)

It's ok if you don't care enough to fix this, me neither. :P

rsekman commented 2 months ago

That warning seems to be due to an upstream change in urwid. GridFlow is now a flow widget and urwid now computes its width as maxcols = N * width + (N-1) h_sep, where N is the number of children. For an empty GridFlow we get maxcols = -h_sep. I think this is an issue in urwid; creating a container widget with no children should not raise a warning.

rsekman commented 2 months ago

I've reported this upstream.

rsekman commented 2 months ago

Fixed as of urwid 2.6.14

rndusr commented 2 months ago

Great!

Thanks again for doing my job. stig would be a lot deader without you.