Tables and spacing for empty rows/columns

[Federico Mena Quintero <federico ximian com>]

I just noticed that GtkTable requests and allocates size for rows and
columns that are empty, or that have no visible widgets in them.  This
leads to surprising spacings in the end.

For example, say we have a 3x3 table like this, and the table has a
certain row spacing.

  child11    child12     child13
  child21    child22     child23
  child31    child32     child33

If I hide all the widgets in the second row, this gets drawn:

  child11    child12     child13
  [gap equal to row1.spacing + row2.spacing]
  child31    child32     child33

When I just expected

  child11    child12     child13
  [gap equal to row1.spacing]
  child31    child32     child33

The result is worse if some of the rows you hide are adjacent to the top
or bottom of the table.  If you take the original table above and hide
rows 1 and 3, then you get this:

  widgets above
  [gap equal to row1.spacing]
  child21    child22     child23
  [gap equal to row2.spacing + row3.spacing]
  widgets below

Both gaps are actually larger, as visually they include the spacing of
the table's parent container (i.e. the "normal" spacing between the
widgets outside the table and the table itself).

I think it would be better to have GtkTable *not* add the spacings for
rows/columns that are empty.  This doesn't look terribly hard to
implement:

* GtkTableRowCol already has an 'empty' field; this gets used only
during the size allocation phase.

* We could use that 'empty' field as well during the size requisition
phase.

* In gtk_table_size_request(), there is an unconditional

  for (row = 0; row + 1 < table->nrows; row++)
    requisition->height += table->rows[row].spacing;

We could make that "+=" conditional on whether the row is empty.

* Similarly, where gtk_table_size_allocate_phase2() does

  y += table->rows[row].spacing

that could be made conditional on the row's emptiness.

Backstory:  in Evolution we have a bunch of widgets for the preferences
dialog, and we would like to hide some of those widgets when Evo is
running in Express mode.  Read that as "we have too many stupid options,
and we are experimenting with reducing their number".  But just hiding
the widgets produces incorrect spacing.

Opinions?  Should I just patch it and see how things look?

  Federico