Fix Columns sizing and pack behavior (#846)

* Always support FLOW for Columns sizing if not BOX-only:
  Use padding/dropping logic.
* Use the same logic for sizing calculation as for render.
* Fix `rows` method in case of FIXED only widgets present.
* `pack` and `render` should produce the same size if some widgets skipped.

Fix: #838

Co-authored-by: Aleksei Stepanov <alekseis@nvidia.com>

tests/test_columns.py

@@ -80,13 +80,40 @@ def test_basic_sizing(self) -> None:
 
         with self.subTest("FIXED only"):
             widget = urwid.Columns(((urwid.PACK, fixed_only),))
-            self.assertEqual(frozenset((urwid.FIXED,)), widget.sizing())
+            self.assertEqual(frozenset((urwid.FLOW, urwid.FIXED)), widget.sizing())
             cols, rows = 3, 3
             self.assertEqual((cols, rows), widget.pack(()))
             canvas = widget.render(())
             self.assertEqual(cols, canvas.cols())
             self.assertEqual(rows, canvas.rows())
 
+        with self.subTest("FIXED only use FLOW methods - drop & replace by solid"):
+            widget = urwid.Columns(((urwid.PACK, fixed_only),))
+            self.assertEqual(frozenset((urwid.FLOW, urwid.FIXED)), widget.sizing())
+            cols, rows = 2, 1
+            self.assertEqual((cols, rows), widget.pack((cols,)))
+            canvas = widget.render((cols,))
+            self.assertEqual(cols, canvas.cols())
+            self.assertEqual(rows, canvas.rows())
+            self.assertEqual("  ", str(canvas))
+
+        with self.subTest("FIXED only use FLOW methods - add empty space"):
+            widget = urwid.Columns(((urwid.PACK, fixed_only),))
+            self.assertEqual(frozenset((urwid.FLOW, urwid.FIXED)), widget.sizing())
+            cols, rows = 5, 3
+            self.assertEqual((cols, rows), widget.pack((cols,)))
+            canvas = widget.render((cols,))
+            self.assertEqual(cols, canvas.cols())
+            self.assertEqual(rows, canvas.rows())
+            self.assertEqual(
+                (
+                    "┌─┐  ",
+                    "│ │  ",
+                    "└─┘  ",
+                ),
+                canvas.decoded_text,
+            )
+
         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.FLOW, urwid.FIXED)), widget.sizing())
@@ -153,8 +180,7 @@ def test_basic_sizing(self) -> None:
             )
 
             cols, rows = 30, 1
-            with self.assertRaises(AttributeError):
-                widget.pack((cols,))
+            self.assertEqual((cols, rows), widget.pack((cols,)))
 
             canvas = widget.render((cols,))
             self.assertEqual(rows, canvas.rows())
@@ -241,6 +267,59 @@ def sizing(self) -> frozenset[urwid.Sizing]:
                 str(ctx.warnings[0].message),
             )
 
+    def test_pack_render_skip_widget(self):
+        widget = urwid.Columns(((0, urwid.Text("Ignore\nme")), (urwid.Text("Count"))))
+        cols, rows = 5, 1
+        self.assertEqual((cols, rows), widget.pack(()))
+        self.assertEqual((cols, rows), widget.pack((cols,)))
+        self.assertEqual("Count", str(widget.render(())))
+        self.assertEqual("Count", str(widget.render((cols,))))
+
+    def test_pack_flow_with_fixed_item(self):
+        widget = urwid.Columns(
+            (
+                (urwid.PACK, urwid.BigText("3", urwid.Thin3x3Font())),
+                (urwid.Text("Multi\nline\ntext\n???")),
+            ),
+            dividechars=1,
+        )
+        with self.subTest("Skip"):
+            self.assertEqual(3, widget.rows((3,)))
+            self.assertEqual((3, 3), widget.pack((3,)))
+            self.assertEqual(
+                (
+                    "┌─┐",
+                    " ─┤",
+                    "└─┘",
+                ),
+                widget.render((3,)).decoded_text,
+            )
+        with self.subTest("Fit all"):
+            self.assertEqual(4, widget.rows((9,)))
+            self.assertEqual((9, 4), widget.pack((9,)))
+            self.assertEqual(
+                (
+                    "┌─┐ Multi",
+                    " ─┤ line ",
+                    "└─┘ text ",
+                    "    ???  ",
+                ),
+                widget.render((9,)).decoded_text,
+            )
+        with self.subTest("Use SolidCanvas instead"):
+            self.assertEqual(1, widget.rows((2,)))
+            self.assertEqual((2, 1), widget.pack((2,)))
+            self.assertEqual(("  ",), widget.render((2,)).decoded_text)
+
+    def test_pack_render_empty_widget(self):
+        widget = urwid.Columns(())
+        self.assertEqual(frozenset((urwid.FLOW, urwid.BOX)), widget.sizing())
+        cols, rows = 10, 1
+        self.assertEqual((cols, rows), widget.pack((cols,)))
+        canvas = widget.render((cols,))
+        self.assertEqual(rows, canvas.rows())
+        self.assertEqual(("          ",), canvas.decoded_text)
+
     def test_not_a_widget(self):
         class NotAWidget:
             __slots__ = ("name", "symbol")

urwid/widget/columns.py

@@ -93,9 +93,9 @@ def sizing(self) -> frozenset[Sizing]:
         >>> Columns(((5, SolidFill("#")), SolidFill("*")), box_columns=(0, 1))
         <Columns box widget (2 items) focus_column=0>
 
-        # FIXED only -> FIXED
+        # FIXED only -> FIXED (and FLOW via drop/expand)
         >>> Columns(((WHSettings.PACK, BigText("1", font)),))
-        <Columns fixed widget (1 item)>
+        <Columns fixed/flow widget (1 item)>
 
         # Invalid sizing combination -> use fallback settings (and produce warning)
         >>> Columns(((WHSettings.PACK, SolidFill("#")),))
@@ -184,6 +184,7 @@ def sizing(self) -> frozenset[Sizing]:
                 supported.add(Sizing.FLOW)
 
             if has_fixed and not block_fixed:
+                supported.add(Sizing.FLOW)
                 supported.add(Sizing.FIXED)
 
         if not supported:
@@ -945,11 +946,17 @@ def get_column_sizes(
                 box.append(i)
 
             elif Sizing.FLOW in w_sizing:
-                heights[i] = widget.rows((width,), focus and i == self.focus_position)
+                if width > 0:
+                    heights[i] = widget.rows((width,), focus and i == self.focus_position)
+                else:
+                    heights[i] = 0
                 w_h_args[i] = (width,)
 
             elif size_kind == WHSettings.PACK:
-                heights[i] = widget.pack((), focus and i == self.focus_position)[1]
+                if width > 0:
+                    heights[i] = widget.pack((), focus and i == self.focus_position)[1]
+                else:
+                    heights[i] = 0
                 w_h_args[i] = ()
 
             else:
@@ -1176,14 +1183,10 @@ def rows(self, size: tuple[int] | tuple[int, int], focus: bool = False) -> int:
 
         see :meth:`Widget.rows` for details
         """
-        widths = self.column_widths(size, focus)
-
-        rows = 1
-        for i, (mc, (w, (_t, _n, b))) in enumerate(zip(widths, self.contents)):
-            if b or mc <= 0:
-                continue
-            rows = max(rows, w.rows((mc,), focus=focus and self.focus_position == i))
-        return rows
+        _, heights, _ = self.get_column_sizes(size, focus)
+        if heights:
+            return max(1, *heights)
+        return 1
 
     def keypress(
         self,