Skip to content

Commit

Permalink
BUG: Columns support correct BOX render only if ALL BOX
Browse files Browse the repository at this point in the history 
Marking of BOX by 1 widget is incorrect.
Remove incorrect test scenarios and cover issue.
  • Loading branch information
@penguinolog
penguinolog committed Jan 15, 2024
1 parent 26c39ff commit 62f19d3
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 50 deletions.
73 changes: 43 additions & 30 deletions tests/test_columns.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ def test_basic_sizing(self) -> None:
fixed_only = urwid.BigText("0", urwid.Thin3x3Font())
flow_fixed = urwid.Text("text")

with self.subTest("No sizing calculation possible"), self.assertWarns(urwid.widget.ColumnsWarning) as ctx:
widget = urwid.Columns(((urwid.PACK, fixed_only), box_only), box_columns=(1,))
self.assertEqual(frozenset((urwid.BOX, urwid.FLOW)), widget.sizing())

for description, kwargs in (
("BOX-only widget", {"widget_list": (box_only,)}),
('BOX-only widget with "get height from max"', {"widget_list": (box_only,), "box_columns": (0,)}),
("No FLOW - BOX only", {"widget_list": (box_only, urwid.SolidFill(" ")), "box_columns": (0, 1)}),
("BOX-only enforced by BOX-only widget", {"widget_list": (box_only, flow_only)}),
("GIVEN BOX only -> BOX only", {"widget_list": ((5, box_only),), "box_columns": (0,)}),
("Corner case: BOX only", {"widget_list": (box_only, flow_only, box_only), "box_columns": (0,)}),
):
with self.subTest(description):
widget = urwid.Columns(**kwargs)
Expand All @@ -47,9 +49,9 @@ def test_basic_sizing(self) -> None:
self.assertEqual(cols, canvas.cols())
self.assertEqual(rows, canvas.rows())

with self.subTest('BOX/FLOW allowed by "box_columns"'):
with self.subTest('BOX/FLOW by "box_columns": can be rendered as box only as fallback'):
widget = urwid.Columns((flow_only, box_only), box_columns=(1,))
self.assertEqual(frozenset((urwid.BOX, urwid.FLOW)), widget.sizing())
self.assertEqual(frozenset((urwid.FLOW,)), widget.sizing())
cols, rows = 2, 1
self.assertEqual((cols, rows), widget.pack((cols,)))
canvas = widget.render((cols,))
Expand Down Expand Up @@ -85,9 +87,9 @@ def test_basic_sizing(self) -> None:
self.assertEqual(cols, canvas.cols())
self.assertEqual(rows, canvas.rows())

with self.subTest("GIVEN BOX + FLOW/FIXED - support all"):
with self.subTest("GIVEN BOX + FLOW/FIXED, but other widgets do not support box"):
widget = urwid.Columns((flow_fixed, (3, box_only)), box_columns=(1,))
self.assertEqual(frozenset((urwid.BOX, urwid.FLOW, urwid.FIXED)), widget.sizing())
self.assertEqual(frozenset((urwid.FLOW, urwid.FIXED)), widget.sizing())
cols, rows = 7, 1
self.assertEqual((cols, rows), widget.pack(()))
canvas = widget.render(())
Expand Down Expand Up @@ -126,7 +128,11 @@ def test_basic_sizing(self) -> None:
str(ctx.warnings[0].message),
)

with self.subTest('BOX not added to "box_columns" but widget handled as FLOW'):
with self.subTest(
'BOX not added to "box_columns" but widget handled as FLOW',
), self.assertWarns(
urwid.widget.ColumnsWarning,
) as ctx:
self.maxDiff = None
contents = (
(urwid.WEIGHT, 1, urwid.SolidFill()),
Expand All @@ -136,34 +142,41 @@ def test_basic_sizing(self) -> None:
(urwid.WEIGHT, 1, urwid.SolidFill()),
)
widget = urwid.Columns(contents)
self.assertEqual(frozenset((urwid.BOX,)), widget.sizing())

self.assertEqual(frozenset((urwid.BOX, urwid.FLOW)), widget.sizing())

self.assertEqual(
"Columns widget contents flags not allow to determine supported render kind:\n"
"BOX|WH_WEIGHT,FLOW|FIXED|WH_GIVEN\n"
"Using fallback hardcoded BOX|FLOW sizing kind.",
str(ctx.warnings[0].message),
)

cols, rows = 30, 1
with self.assertRaises(urwid.WidgetError):
with self.assertRaises(AttributeError):
widget.pack((cols,))
with self.assertWarns(urwid.widget.ColumnsWarning) as ctx:
canvas = widget.render((cols,))
self.assertEqual(rows, canvas.rows())
self.assertEqual(cols, canvas.cols())
self.assertEqual([b" < OK > < Cancel > "], canvas.text)
self.assertEqual(
"Widgets in columns [0, 2, 4] "
f"({[elem[-1] for elem in contents[:6:2]]}) "
f'are BOX widgets not marked "box_columns" '
f"while FLOW render is requested (size=(30,))",
str(ctx.warnings[0].message),
)

canvas = widget.render((cols,))
self.assertEqual(rows, canvas.rows())
self.assertEqual(cols, canvas.cols())
self.assertEqual([b" < OK > < Cancel > "], canvas.text)
self.assertEqual(
"Widgets in columns [0, 2, 4] "
f"({[elem[-1] for elem in contents[:6:2]]}) "
f'are BOX widgets not marked "box_columns" '
f"while FLOW render is requested (size=(30,))",
str(ctx.warnings[2].message),
)

self.assertEqual("OK", widget.focus.label)
with self.assertWarns(urwid.widget.ColumnsWarning) as ctx:
self.assertIsNone(widget.keypress((cols,), "right"))
self.assertEqual(
"Widgets in columns [0, 2, 4] "
f"({[elem[-1] for elem in contents[:6:2]]}) "
f'are BOX widgets not marked "box_columns" '
f"while FLOW render is requested (size=(30,))",
str(ctx.warnings[0].message),
)
self.assertIsNone(widget.keypress((cols,), "right"))
self.assertEqual(
"Widgets in columns [0, 2, 4] "
f"({[elem[-1] for elem in contents[:6:2]]}) "
f'are BOX widgets not marked "box_columns" '
f"while FLOW render is requested (size=(30,))",
str(ctx.warnings[3].message),
)
self.assertEqual("Cancel", widget.focus.label)

def test_pack_render_fixed(self) -> None:
Expand Down
45 changes: 25 additions & 20 deletions urwid/widget/columns.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def sizing(self) -> frozenset[Sizing]:
Backward compatibility rules:
* GIVEN BOX -> Allow BOX
FIXED can be only if no BOX and no strict FLOW.
BOX can be only if ALL widgets support BOX.
FIXED can be only if no BOX without "box_columns" flag and no strict FLOW.
>>> from urwid import BigText, Edit, SolidFill, Text, Thin3x3Font
>>> font = Thin3x3Font()
Expand All @@ -73,17 +74,9 @@ def sizing(self) -> frozenset[Sizing]:
>>> Columns((Edit(),))
<Columns selectable flow widget>
# BOX-only enforced by BOX-only widget
>>> Columns((Edit(), SolidFill("#")))
<Columns selectable box widget>
# BOX/FLOW allowed by "box_columns"
# FLOW allowed by "box_columns"
>>> Columns((Edit(), SolidFill("#")), box_columns=(1,))
<Columns selectable box/flow widget>
# Corner case: BOX only
>>> Columns((Edit(), SolidFill("#"), SolidFill("#")), box_columns=(1,))
<Columns selectable box widget>
<Columns selectable flow widget>
# FLOW/FIXED
>>> Columns((Text("T"),))
Expand All @@ -97,10 +90,6 @@ def sizing(self) -> frozenset[Sizing]:
>>> Columns(((5, SolidFill("#")), SolidFill("*")), box_columns=(0, 1))
<Columns box widget>
# We can everything: GIVEN BOX + FLOW/FIXED
>>> Columns(((5, SolidFill("#")), (3, Text("T"))), box_columns=(0,))
<Columns widget>
# FIXED only -> FIXED
>>> Columns(((WHSettings.PACK, BigText("1", font)),))
<Columns fixed widget>
Expand All @@ -122,6 +111,8 @@ def sizing(self) -> frozenset[Sizing]:
flow_fixed = _ContainerElementSizingFlag.FLOW | _ContainerElementSizingFlag.FIXED
given_box = _ContainerElementSizingFlag.BOX | _ContainerElementSizingFlag.WH_GIVEN

flags: set[_ContainerElementSizingFlag] = set()

for idx, (widget, (size_kind, _size_weight, is_box)) in enumerate(self.contents):
w_sizing = widget.sizing()

Expand Down Expand Up @@ -160,12 +151,10 @@ def sizing(self) -> frozenset[Sizing]:
)
return frozenset((Sizing.BOX, Sizing.FLOW))

if flag & _ContainerElementSizingFlag.BOX:
supported.add(Sizing.BOX)
flags.add(flag)

if not (is_box or flag & flow_fixed):
strict_box = True
break
if flag & _ContainerElementSizingFlag.BOX and not (is_box or flag & flow_fixed):
strict_box = True

if flag & _ContainerElementSizingFlag.FLOW:
has_flow = True
Expand All @@ -174,13 +163,29 @@ def sizing(self) -> frozenset[Sizing]:
elif flag & given_box != given_box:
block_fixed = True

if all(flag & _ContainerElementSizingFlag.BOX for flag in flags):
# Only if ALL widgets can be rendered as BOX, widget can be rendered as BOX.
# Hacky "BOX" render for FLOW-only is still present,
# due to incorrected implementation can be used by downstream
supported.add(Sizing.BOX)

if not strict_box:
if has_flow:
supported.add(Sizing.FLOW)

if has_fixed and not block_fixed:
supported.add(Sizing.FIXED)

if not supported:
warnings.warn(
f"Columns widget contents flags not allow to determine supported render kind:\n"
f"{','.join(flag.name for flag in flags)}\n"
f"Using fallback hardcoded BOX|FLOW sizing kind.",
ColumnsWarning,
stacklevel=3,
)
return frozenset((Sizing.BOX, Sizing.FLOW))

return frozenset(supported)

def __init__(
Expand Down

0 comments on commit 62f19d3

Please sign in to comment.