Grouping Desktop moved to kdereview

[Albert Astals Cid]

A Dijous, 22 de juliol de 2010, Giulio Camuffo va escriure:
> Hi all KDE devs,
> 
> I've moved to kdereview/plasma/containments/ Grouping Desktop, my pet
> project which I've been working on these last months.
> It is a Desktop and a Panel Plasma Containments, in which I added the
> support for grouping Applets in grids, in tabs, in a stack or in simple
> frames.
> 
> I've been developing this since the last months of 2009 and i fell now it
> is, while not bug free, quite stable and clean.
> 
> On
> http://websvn.kde.org/trunk/playground/base/plasma/containments/groupingdes
> ktop/README?view=markup there is a quite comprehensive README so I'm not
> repeating all of that here. Entry on kde-look:
> http://kde-look.org/content/show.php?content=116926 I uploaded a video on
> youtube, if you want to have a quich shot (even if that is old) :
> http://www.youtube.com/watch?v=SL0PU2-u0E8
> 
> Probably the best target for this is kdeplasma-addons, event if I don't
> hide that I'd like to see it in kdebase. :)

Some i18n comments

  i18n("Add a new ") + AbstractGroup::prettyName(group)
should be
  i18n("Add a new %1", AbstractGroup::prettyName(group))
and ideally should be a i18nc explainig what %1 is

  i18n("General")
needs to be i18nc explaining what this General is

  tabs << "New Tab";
sounds like something that should be i18n'ed

Your Messages.sh does not include the ui files so they can not be properly 
translated


Some code efficiency comments, in lib/groupfactory.cpp you do

AbstractGroup *GroupFactory::load(const QString &name, QGraphicsItem *parent)
{
    QList<GroupInfo> giList = m_groups->keys();
    foreach (const GroupInfo &gi, giList) {
        if (gi.name == name) {
            return (*m_groups->value(gi))(parent);
        }
    }

    return 0;
}

and 

AbstractGroup *GroupFactory::load(const QString &name, QGraphicsItem *parent)
{
    GroupInfo gi(name);
    AbstractGroup *(*)(QGraphicsItem *) foo = m_groups->value(gi);
    if (foo) return (foo)(parent);
    else return 0;
}

should be the same and faster (since it does not to go through all the map) 
(i'm not totally sure if foo is correctly declared but i hope you get the 
idea)

Also
    QList<QGraphicsWidget *> children = childrenToBeMoved.keys();
    foreach (QGraphicsWidget *child, children) {
        int tab = childrenToBeMoved.value(child);
        child->setParentItem(m_tabBar->tabAt(tab)->graphicsItem());
        m_children.insert(child, tab);
    }
should be rewritten using iterators to avoid doing lots and lots of unneded 
accesses to the map.

Albert

> 
> Grettings, Giulio