Skip to content

Commit

Permalink
ScrollBar: fully support __length_hint__ not Sized (urwid#863)
Browse files Browse the repository at this point in the history
* `ScrollBar`: fully support `__length_hint__` not `Sized`

* Convert `__len__` and `__length_hint__` of `ListBox` to properties:
  `AttributeError` will be properly handled by `getattr` / `hasattr`
* Reset to the absolute render way in case of the all contents are expected to be rendered
* better handling of decorated widgets for scrolling

* Update browse example to make scrollbar usage more human-friendly

---------

Co-authored-by: Aleksei Stepanov <alekseis@nvidia.com>
  • Loading branch information
penguinolog and Aleksei Stepanov committed Mar 18, 2024
1 parent c63eb2d commit bc7f44b
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 20 deletions.
13 changes: 10 additions & 3 deletions examples/browse.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,14 @@ def __init__(self) -> None:
self.listbox.offset_rows = 1
self.footer = urwid.AttrMap(urwid.Text(self.footer_text), "foot")
self.view = urwid.Frame(
urwid.AttrMap(urwid.ScrollBar(self.listbox), "body"),
urwid.AttrMap(
urwid.ScrollBar(
self.listbox,
thumb_char=urwid.ScrollBar.Symbols.FULL_BLOCK,
trough_char=urwid.ScrollBar.Symbols.LITE_SHADE,
),
"body",
),
header=urwid.AttrMap(self.header, "head"),
footer=self.footer,
)
Expand Down Expand Up @@ -379,9 +386,9 @@ def escape_filename_sh_ansic(name: str) -> str:
SPLIT_RE = re.compile(r"[a-zA-Z]+|\d+")


def alphabetize(s):
def alphabetize(s: str) -> list[str]:
L = []
for isdigit, group in itertools.groupby(SPLIT_RE.findall(s), key=lambda x: x.isdigit()):
for isdigit, group in itertools.groupby(SPLIT_RE.findall(s), key=str.isdigit):
if isdigit:
L.extend(("", int(n)) for n in group)
else:
Expand Down
61 changes: 57 additions & 4 deletions tests/test_scrollable.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
from __future__ import annotations

import string
import typing
import unittest

import urwid

if typing.TYPE_CHECKING:
from collections.abc import Iterable

LGPL_HEADER = """
Copyright (C) <year> <name of author>
Expand Down Expand Up @@ -264,15 +268,13 @@ def test_relative_non_selectable(self):

widget.keypress(reduced_size, "end")

# Here we have ListBox issue: "end" really scroll not to the "dead end"
# in case of the bottom focus position is multiline
self.assertEqual(
(
"GNU Lesser General Public ",
"License along with this library; if ",
"not, write to the Free Software ",
"Foundation, Inc., 51 Franklin Street, ",
"Fifth Floor, Boston, MA 02110-1301 █",
"Fifth Floor, Boston, MA 02110-1301 ",
"USA █",
),
widget.render(reduced_size).decoded_text,
)
Expand Down Expand Up @@ -344,3 +346,54 @@ def test_large_selectable(self):
widget.keypress(reduced_size, "down")

self.assertEqual(pos_1_down_rendered, widget.render(reduced_size).decoded_text)

def test_hinted_len(self):
class HintedWalker(urwid.ListWalker):
def __init__(self, items: Iterable[str]) -> None:
self.items: tuple[str] = tuple(items)
self.focus = 0
self.requested_numbers: set[int] = set()

def __length_hint__(self) -> int:
return len(self.items)

def __getitem__(self, item: int) -> urwid.Text:
self.requested_numbers.add(item)
return urwid.Text(self.items[item])

def set_focus(self, item: int) -> None:
self.focus = item

def next_position(self, position: int) -> int:
if position + 1 < len(self.items):
return position + 1
raise IndexError

def prev_position(self, position: int) -> int:
if position - 1 >= 0:
return position - 1
raise IndexError

widget = urwid.ScrollBar(urwid.ListBox(HintedWalker((f"Line {idx:02}") for idx in range(1, 51))))
size = (10, 10)
widget.original_widget.focus_position = 19
self.assertEqual(
(
"Line 16 ",
"Line 17 ",
"Line 18 ",
"Line 19 █",
"Line 20 █",
"Line 21 ",
"Line 22 ",
"Line 23 ",
"Line 24 ",
"Line 25 ",
),
widget.render(size).decoded_text,
)
self.assertNotIn(
30,
widget.original_widget.body.requested_numbers,
"Requested index out of range [0, last shown]. This means not relative scroll bar built.",
)
14 changes: 11 additions & 3 deletions urwid/widget/listbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

from __future__ import annotations

import operator
import typing
import warnings
from collections.abc import Iterable, Sized
Expand Down Expand Up @@ -443,11 +444,18 @@ def _set_body(self, body):
)
self.body = body

def __len__(self) -> int:
@property
def __len__(self) -> Callable[[], int]:
if isinstance(self._body, Sized):
return len(self._body)
return self._body.__len__
raise AttributeError(f"{self._body.__class__.__name__} is not Sized")

@property
def __length_hint__(self) -> Callable[[], int]: # pylint: disable=invalid-length-hint-returned
if isinstance(self._body, (Sized, EstimatedSized)):
return lambda: operator.length_hint(self._body)
raise AttributeError(f'{self._body.__class__.__name__} is not Sized and do not implement "__length_hint__"')

def calculate_visible(
self,
size: tuple[int, int],
Expand Down Expand Up @@ -655,7 +663,7 @@ def rows_max(self, size: tuple[int, int] | None = None, focus: bool = False) ->

def require_relative_scroll(self, size: tuple[int, int], focus: bool = False) -> bool:
"""Widget require relative scroll due to performance limitations of real lines count calculation."""
return isinstance(self._body, Sized) and (size[1] * 3 < len(self))
return isinstance(self._body, (Sized, EstimatedSized)) and (size[1] * 3 < operator.length_hint(self.body))

def get_first_visible_pos(self, size: tuple[int, int], focus: bool = False) -> int:
self._check_support_scrolling()
Expand Down
32 changes: 22 additions & 10 deletions urwid/widget/scrollable.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,6 @@ def rows_max(self, size: tuple[int, int] | None = None, focus: bool = False) ->
class SupportsRelativeScroll(WidgetProto, Protocol):
"""Relative scroll-specific methods."""

def __len__(self) -> int: ...

def require_relative_scroll(self, size: tuple[int, int], focus: bool = False) -> bool: ...

def get_first_visible_pos(self, size: tuple[int, int], focus: bool = False) -> int: ...
Expand Down Expand Up @@ -516,18 +514,32 @@ def render_for_scrollbar() -> Canvas:
ow = self._original_widget
ow_base = self.scrolling_base_widget

if isinstance(ow, SupportsRelativeScroll) and ow.require_relative_scroll(size, focus):
if len(ow) == ow.get_visible_amount(size, focus):
# Canvas fits without scrolling - no scrollbar needed
return render_no_scrollbar()
# Use hasattr instead of protocol: hasattr will return False in case of getattr raise AttributeError
# Use __length_hint__ first since it's less resource intensive
use_relative = (
isinstance(ow_base, SupportsRelativeScroll)
and any(hasattr(ow_base, attrib) for attrib in ("__length_hint__", "__len__"))
and ow_base.require_relative_scroll(size, focus)
)

if use_relative:
# `operator.length_hint` is Protocol (Spec) over class based and can end false-negative on the instance
# use length_hint-like approach with safe `AttributeError` handling
ow_len = getattr(ow_base, "__len__", getattr(ow_base, "__length_hint__", lambda: 0))()
ow_canv = render_for_scrollbar()
visible_amount = ow.get_visible_amount(ow_size, focus)
visible_amount = ow_base.get_visible_amount(ow_size, focus)
pos = ow_base.get_first_visible_pos(ow_size, focus)
posmax = len(ow) - visible_amount
thumb_weight = min(1.0, visible_amount / max(1, len(ow)))

else:
# in the case of estimated length, it can be smaller than real widget length
ow_len = max(ow_len, visible_amount, pos)
posmax = ow_len - visible_amount
thumb_weight = min(1.0, visible_amount / max(1, ow_len))

if ow_len == visible_amount:
# Corner case: formally all contents indexes should be visible, but this does not mean all rows
use_relative = False

if not use_relative:
ow_rows_max = ow_base.rows_max(size, focus)
if ow_rows_max <= maxrow:
# Canvas fits without scrolling - no scrollbar needed
Expand Down

0 comments on commit bc7f44b

Please sign in to comment.