Re: [PATCH 1/4] Popup var inspector size requisition code rework

[Dodji Seketeli <dodji seketeli org>]

Hello Kalev,

This is a great patch, thank you!  I just have an extremely picky nit
comment and some questions below.

[ By the way, I am CC-ing you even though you are subscribed to the
  mailing list, because you did set the CC in your original email.  If
  you didn't want me to CC you as well as replying to the mailing list,
  please let me know, so that I adjust my future replies accordingly.  ]

[...]

> 	* src/uicommon/nmv-popup-tip.cc (PopupTip::Priv::Priv): Set the
> 	window as not resizable so its size would shrink back along with
> 	its contents.

This works nicely for the vertical dimension.  I.e, it shrinks back
/vertically/ along with its content.  But it doesn't shrink back
/horizontally/, for me.  I wonder if there is something to do for this.
Your patch is an improvement over what's in there already, so I agree
with that particular change.  I am just wondering if there is something
to do for the horizontal dimension as well.

[...]

>  src/persp/dbgperspective/nmv-dbg-perspective.cc |  142 ++++++++---------------
>  src/uicommon/nmv-popup-tip.cc                   |    2 +-
>  2 files changed, 48 insertions(+), 96 deletions(-)

> +    struct PopupScrolledWindow : Gtk::ScrolledWindow {
> +        // Try to keep the height of the ScrolledWindow the same as the widget
> +        // it contains, so that the user doesn't have to scroll. This has a
> +        // limit though. When we reach the screen border, we don't want the
> +        // container to grow past the border. At that point, the user will have
> +        // to scroll.
> +        virtual void on_size_request (Gtk::Requisition *req)
> +        {

The comment above should be made with '///' instead of just '//',
because IIRC, as it's the comment of a function, our doxygen's setup
needs it to be '///' to recognize is at the comment of a function.

[...]

> +            const Gtk::Widget *child = get_child ();
> +            THROW_IF_FAIL (child);
> +            Gtk::Requisition child_req = child->size_request ();
> +
> +            // If the height of the container is too big so that
> +            // it overflows the max usable height, clip it.
> +            if (child_req.height > max_height) {
> +                req->height = max_height;
> +            } else {
> +                req->height = child_req.height;
> +            }

Now that we are doing this on the scrolled window (the container)
rather than on the popup variable inspector itself, it seems to me
that it is the height of the scrolled window that we want to stay
smaller than max_height, not necessarily the height of its child
widget, or am I missing something ?

FWIW, I have tested the code snipped below and it seems to behave
fine, unless I have missed a corner case:

            // If the height of the container is too big so that
            // it overflows the max usable height, clip it.
            if (req.height > max_height) {
                req->height = max_height;
            } else {
                req->height = child_req.height;
            }

Thanks.

-- 
		Dodji