[Qgis-developer] Legend group API fix

[Sandro Santilli]

FYI, as of 677095f7911cb0e2af91004cf37dfc988d75eded I changed
the approach by keeping the current (overcomplex IMHO) interpretation
of the integer version of "groupIndex" and simply bending the
return from addGroup to this interpretation (rather than to the
QModelIndex::row() as it's been doing now).

It worked effectively for my case:
http://hub.qgis.org/issues/6591
And doesn't change any API.

I was thinking that maybe for a next step we could be
using the qint64 "internalId" provided by QModelIndex class:
http://doc.qt.digia.com/qt/qmodelindex.html#internalId

It would break the ABI, but the API wouldn't be much different
(64bit integer vs. whatever integer you happen to have with "int").
Or, we could cast the qint64 to an int and hope we'll never have
a too large qmodelindex (could we reach that limit in just a legend?).

Anyway, looking forward for Martin refactoring

--strk;

On Tue, Dec 11, 2012 at 05:25:57PM +0100, Sandro Santilli wrote:
> On Tue, Dec 11, 2012 at 07:42:08AM -0800, Massimo Endrighi wrote:
> > Sandro Santilli-2 wrote
> > > I think Martin ideas is sensible and I do see the code is going
> > > in that direction. But even if the classes are decoupled from
> > > the QT tree widget we'll need a way to identify groups, as much
> > > as we do with layers (which do have an ID). I've no problem with
> > > all elements (be them layers or groups or whatever) share the
> > > same "id space" but each one should have an identifier which
> > > remains valid for the whole lifetime of that object.
> > 
> > Please keep in mind that it is also necessary to be able to:
> > - move/put a layer into a sub group (not only a top level item as it was in
> > 1.8)
> > - remove a sub group (not only a top level item as it was in 1.8)
> > - move a layer before or after another layer inside the same group
> > - walk thought the legend hierarchy even when there are sub groups and
> > multiple group with the same name (the groupLayerRelationship is unusable in
> > some circumstances)
> 
> I'm a fan of incremental refactoring, to minimize regressions.
> In this case I don't think the current API, nor the 1.8 one allowed
> for moving a layer before or after another one inside or outside a group,
> but in all APIs the so-called "groupIndex" seems to be effectively used
> as an identifier fo the group:
> 
>     //! Add a new group
>     //! @note added parentIndex parameter in 1.7
>     virtual int addGroup( QString name, bool expand, int parentIndex ) = 0;
> 
>     //! Remove group on index
>     virtual void removeGroup( int groupIndex ) =0;
> 
>     //! Move a layer to a group
>     virtual void moveLayer( QgsMapLayer * layer, int groupIndex ) =0;
> 
> There's even:
> 
>     //! Check if a group exists
>     //! @note added in 1.5
>     virtual bool groupExists( int groupIndex ) =0;
> 
> All these functions take an "int" but all assume it identifies a group,
> not a location in a tree.
> 
> I'm happy with the interface extending to allow more fine-tuned access to
> the tree, but it must also be possile to effectively use the above functions
> the way they are documented to work, and adding the concept of a
> "group identifier" (basically replacing "groupIndex" with "groupID")
> keeps the same interface just making it unambiguos.
> 
> Methods dealing with the tree I think should not contain the "Layer" or
> "Group" names, but only "Tree" and "Element"...
> 
> --strk;