Grouping Desktop moved to kdereview

[Giulio Camuffo]

In data venerdì 23 luglio 2010 20:24:52, Albert Astals Cid ha scritto:
> 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

Thanks for the remarks, fixed.

Giulio