gtkmm4: Box::pack_start()/pack_end()

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

gtkmm4: Box::pack_start()/pack_end()

Murray Cumming-5
The gtk_box_pack_start()/pack_end() API changed slightly,
meaning that our C++ pack_start()/pack_end() convenience overloads can
no longer have a default value for our PackOptions convenience enum:
https://git.gnome.org/browse/gtkmm/commit/?id=b07a05bba3a2e1df8778683a4
7e50d8c8f3fb26b

As the commit message says, that's awkward because the behaviour has
now changed silently, meaning you need to do this to get the old
behaviour (assuming my new implementation works as intended):
https://git.gnome.org/browse/gtkmm/commit/?id=214be98c94d85aa5ac79031fa
a5f995e4b797c26

Thoughts? Should we remove these gtkmm-specific
Box::pack_start()/pack_end() methods anyway? That will still have the
same change-of-behaviour problem for this code:
box.pack_start(child)
but if an application also has code like this it will at least cause a
compiler error that forces the developer to think about it:
box.pack_start(child, Gtk::PACK_SHRINK).


--
Murray Cumming
[hidden email]
www.murrayc.com

_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: gtkmm4: Box::pack_start()/pack_end()

Murray Cumming-5
On Thu, 2017-04-27 at 12:31 +0200, Murray Cumming wrote:

> The gtk_box_pack_start()/pack_end() API changed slightly,
> meaning that our C++ pack_start()/pack_end() convenience overloads
> can
> no longer have a default value for our PackOptions convenience enum:
> https://git.gnome.org/browse/gtkmm/commit/?id=b07a05bba3a2e1df8778683
> a4
> 7e50d8c8f3fb26b
>
> As the commit message says, that's awkward because the behaviour has
> now changed silently, meaning you need to do this to get the old
> behaviour (assuming my new implementation works as intended):
> https://git.gnome.org/browse/gtkmm/commit/?id=214be98c94d85aa5ac79031
> fa
> a5f995e4b797c26

If we had a gtkmm 3.23/34, we could just remove the default value,
forcing people to specify the value in their code, thus making it ready
for gtkmm 4. Such a small API change is acceptable between, for
instance, 3.22 and 3.24, but would be an unpleasant surprise for people
if we did it, for instance, between 3.22.1 and 3.22.2.

> Thoughts? Should we remove these gtkmm-specific
> Box::pack_start()/pack_end() methods anyway? That will still have the
> same change-of-behaviour problem for this code:
> box.pack_start(child)
> but if an application also has code like this it will at least cause
> a
> compiler error that forces the developer to think about it:
> box.pack_start(child, Gtk::PACK_SHRINK).
>
>
--
Murray Cumming
[hidden email]
www.murrayc.com

_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: gtkmm4: Box::pack_start()/pack_end()

Kjell Ahlstedt-2
In reply to this post by Murray Cumming-5
On 2017-04-27 12:31, Murray Cumming wrote:
Thoughts? Should we remove these gtkmm-specific
Box::pack_start()/pack_end() methods anyway? That will still have the
same change-of-behaviour problem for this code:
box.pack_start(child)
but if an application also has code like this it will at least cause a
compiler error that forces the developer to think about it:
box.pack_start(child, Gtk::PACK_SHRINK).


Why does Gtk::PackOptions affect only horizontal expansion and alignment? Look for instance at one of the 3  TreeView demo programs. The ScrolledWindow is expanded horizontally but not vertically. Before the expand and fill parameters were removed from gtk_box_pack_start() the ScrolledWindow was expanded in both directions.

_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: gtkmm4: Box::pack_start()/pack_end()

Murray Cumming-5
On Fri, 2017-04-28 at 08:31 +0200, Kjell Ahlstedt wrote:

> On 2017-04-27 12:31, Murray Cumming wrote:
> > Thoughts? Should we remove these gtkmm-specific
> > Box::pack_start()/pack_end() methods anyway? That will still have
> > the
> > same change-of-behaviour problem for this code:
> > box.pack_start(child)
> > but if an application also has code like this it will at least
> > cause a
> > compiler error that forces the developer to think about it:
> > box.pack_start(child, Gtk::PACK_SHRINK).
> >
> >
>  Why does Gtk::PackOptions affect only horizontal expansion and
> alignment? Look for instance at one of the 3  TreeView demo programs.
> The ScrolledWindow is expanded horizontally but not vertically.
> Before the expand and fill parameters were removed from
> gtk_box_pack_start() the ScrolledWindow was expanded in both
> directions.

Maybe our pack_start/end() implementation should set hexpand/halign or
vexpand/valign depending on the orientation, or always set both.

--
Murray Cumming
[hidden email]
www.murrayc.com

_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: gtkmm4: Box::pack_start()/pack_end()

Kjell Ahlstedt-2
On 2017-04-28 09:08, Murray Cumming wrote:
On Fri, 2017-04-28 at 08:31 +0200, Kjell Ahlstedt wrote:
 Why does Gtk::PackOptions affect only horizontal expansion and
alignment? Look for instance at one of the 3  TreeView demo programs.
The ScrolledWindow is expanded horizontally but not vertically.
Before the expand and fill parameters were removed from
gtk_box_pack_start() the ScrolledWindow was expanded in both
directions.
Maybe our pack_start/end() implementation should set hexpand/halign or
vexpand/valign depending on the orientation, or always set both.

I vote for always setting both, if we will keep Gtk::PackOptions.

_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: gtkmm4: Box::pack_start()/pack_end()

Murray Cumming-5
On Fri, 2017-04-28 at 13:25 +0200, Kjell Ahlstedt wrote:

> On 2017-04-28 09:08, Murray Cumming wrote:
> > On Fri, 2017-04-28 at 08:31 +0200, Kjell Ahlstedt wrote:
> > >  Why does Gtk::PackOptions affect only horizontal expansion and
> > > alignment? Look for instance at one of the 3  TreeView demo
> > > programs.
> > > The ScrolledWindow is expanded horizontally but not vertically.
> > > Before the expand and fill parameters were removed from
> > > gtk_box_pack_start() the ScrolledWindow was expanded in both
> > > directions.
> >
> > Maybe our pack_start/end() implementation should set hexpand/halign
> > or
> > vexpand/valign depending on the orientation, or always set both.
> >
>  I vote for always setting both, if we will keep Gtk::PackOptions.
This fixes that problem with the ScrolledWindow:
https://git.gnome.org/browse/gtkmm/commit/?id=ece3794336e7216c4d25484be
432d5d142ebab1b

but it causes the left-hand treeview (list of examples) in the demo to
not expand vertically, and the "Add item" and "Remove item" buttons in
the "Editable Cells" example now appear to the left instead of filling
up the whole horizontal space.

With the attached patch, things seem better, but then those buttons
expand vertically too.

I guess we need to find out exactly what
halign/hexpand/valign/vexpand/whatever values are really meant to
provide the same behaviour as the previous pack_start(child, expand,
fill):
https://developer.gnome.org/gtk3/stable/GtkBox.html#gtk-box-pack-start

It doesn't look like our old pack_start(child, SHRINK) is the same as
the new gtk_box_pack_start(child).

--
Murray Cumming
[hidden email]
www.murrayc.com

_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list

0001-Box-pack_start-pack_end-options-Avoid-setting-child-.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: gtkmm4: Box::pack_start()/pack_end()

Murray Cumming-5
On Fri, 2017-04-28 at 14:34 +0200, Murray Cumming wrote:

> On Fri, 2017-04-28 at 13:25 +0200, Kjell Ahlstedt wrote:
> > On 2017-04-28 09:08, Murray Cumming wrote:
> > > On Fri, 2017-04-28 at 08:31 +0200, Kjell Ahlstedt wrote:
> > > >  Why does Gtk::PackOptions affect only horizontal expansion and
> > > > alignment? Look for instance at one of the 3  TreeView demo
> > > > programs.
> > > > The ScrolledWindow is expanded horizontally but not vertically.
> > > > Before the expand and fill parameters were removed from
> > > > gtk_box_pack_start() the ScrolledWindow was expanded in both
> > > > directions.
> > >
> > > Maybe our pack_start/end() implementation should set
> > > hexpand/halign
> > > or
> > > vexpand/valign depending on the orientation, or always set both.
> > >
> >
> >  I vote for always setting both, if we will keep Gtk::PackOptions.
>
> This fixes that problem with the ScrolledWindow:
> https://git.gnome.org/browse/gtkmm/commit/?id=ece3794336e7216c4d25484
> be
> 432d5d142ebab1b
>
> but it causes the left-hand treeview (list of examples) in the demo
> to
> not expand vertically, and the "Add item" and "Remove item" buttons
> in
> the "Editable Cells" example now appear to the left instead of
> filling
> up the whole horizontal space.
>
> With the attached patch, things seem better, but then those buttons
> expand vertically too.
>
> I guess we need to find out exactly what
> halign/hexpand/valign/vexpand/whatever values are really meant to
> provide the same behaviour as the previous pack_start(child, expand,
> fill):
> https://developer.gnome.org/gtk3/stable/GtkBox.html#gtk-box-pack-star
> t
>
> It doesn't look like our old pack_start(child, SHRINK) is the same as
> the new gtk_box_pack_start(child).

This seems to explain it:
https://mail.gnome.org/archives/gtk-devel-list/2017-April/msg00048.html

I guess application code must add an extra
box.set_vexpand(false) or box.set_hexpand(false) to get the old
behaviour, but not always.

I guess we should remove our pack_start/pack_end(options) method, but
I'd like to keep it just a little longer, so we can port application
code gradually.

--
Murray Cumming
[hidden email]
www.murrayc.com

_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Loading...