[haiku-commits] Re: r38064 - in haiku/trunk/src: add-ons/disk_systems/bfs add-ons/disk_systems/intel apps/deskbar apps/devices apps/launchbox ...

  • From: Alex Wilson <yourpalal2@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 12 Aug 2010 14:35:38 -0600

On Thu, Aug 12, 2010 at 2:04 PM, Stephan Assmus <superstippi@xxxxxx> wrote:

>
> On 2010-08-12 at 20:38:50 [+0200], Alex Wilson <yourpalal2@xxxxxxxxx>
> wrote:
> > On Thu, Aug 12, 2010 at 12:14 PM, Axel Dörfler
> > <axeld@xxxxxxxxxxxxxxxx>wrote:
> >
> > > yourpalal2@xxxxxxxxx wrote:
> > > >       BView* view = BGroupLayoutBuilder(B_VERTICAL, 5.0)
> > > >               .Add(gridLayout->View())
> > > >               .Add(fCalendarView)
> > > > -             .SetInsets(5.0, 5.0, 5.0, 5.0);
> > > > +             .SetInsets(5.0, 5.0, 5.0, 5.0)
> > > > +             .TopView();
> > > >       groupView->AddChild(view);
> > >
> > > I know I missed the discussion about this, and sorry for that, but what
> > > is the motivation for this "ugliness" in the API? I guess the
> > > BGroupLayoutBuilder is therefore no longer the preferred method of
> > > building group layouts?
> > >
> > > Bye,
> > >    Axel.
> > >
> >
> > That's alright, other people may be wondering as well.
> >
> > I think you're referring to the 'TopView()' call. Once BLayout derives
> from
> > BLayoutItem, then BView::AddChild(someBuilder) becomes ambiguous, but
> > BView::AddChild((BLayout*)someBuilder) will still have the desired
> effect.
> > However, sometimes one might still want to have access to the view that
> > would have been returned by operator BView*(), so I added methods to
> allow
> > that to the builders. Also, at this time, that change lives only in my
> local
> > tree, so while we have no operator BView*(), and operator BGroupLayout*()
> > doesn't yield a BLayoutItem*, we can't simply add builders.
> >
> > An example of how that particular example could be beautified with the
> > templatized layout builders (and some upcoming changes to them), would
> be:
> >
> > BLayoutBuilder::Group<>(groupView)
> >      .Add(gridLayout)
> >      .Add(fCalendarView)
> >      .SetInsets(5.0, 5.0, 5.0, 5.0);
> >
> > which is much nicer looking. However, we could go even prettier with the
> new
> > builders like so:
> >
> > BLayoutBuilder::Group<>(this, B_VERTICAL, 10.0)
> >     .AddGroup(B_VERTICAL, 5.0)
> >         .AddGrid()
> >              // add those things that were added to the grid builder
> above,
> > also capture this layout
> >              // so that we can do 'SetMinColumnWidth'
> >              .End()
> >         .Add(fCalendarView)
> >         .SetInsets(...);
> >
> > Which really much nicer. But anyway, I was mostly trying to just break up
> > all my local changes into some smaller commits (but not too small ;) that
> > can be done separately w/o build breakage.
> > Also, going through the tree fixing things that break with the upcoming
> > change, I've seen some weird layout stuff, and I am planning on going
> > through and beautifying all that once I'm finished my work on the layout
> API
> > (that would include this file).
> >
> > Hope that answers your question :)
>
> Thanks for the explanation! I am not quite sure I follow you completely,
> did
> I understand correctly that the TopView() requirement is just temporary
> until
> you have commited the rest of your changes?
>

Yes and no, once I've committed the rest of my changes it will be fine to
AddChild() a builder directly, but if you wish to assign one to a BView*,
you would still need to use either TopView() or GetTopView(). I guess you
could also use TopLayout()->View(), but that is even uglier. However, you
will also be able to construct all builders with  an explicit
B{Group|Grid|Split}View*, which you could capture before hand. I'm not
really fond of the methods myself, but they seem necessary (at least for the
how the builders are currently used in many places).

So to sum it up, you will be able to get around just fine without using such
methods, but in the case of BView* = SomeBuilderClass (Which won't be
strictly necessary, but some code already works like this). You will have to
use some type of accessor method, because the BView* cast operator will not
be able to co-exist peacefully with the BLayout* cast operator.

These methods could theoretically be removed later, since they are there
only to offer the same functionality as the BView* cast operator, which I
think was probably added so that builders could be passed added directly to
views or other layouts, which will work as expected once BLayout derives
from BLayoutItem. Another argument for removing them is that the AddGroup()
and AddGrid() methods will add a viewless layout (unless you pass in a
BGr{id|oup}View*), and in such a case, they will just return NULL anyway.

Hopefully that was more clear :P

--Alex

Other related posts: