[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Grouping Desktop moved to kdereview
From: Giulio Camuffo <giuliocamuffo () gmail ! com>
Date: 2010-07-24 11:13:16
Message-ID: 201007241313.16700.giuliocamuffo () gmail ! com
[Download RAW message or body]
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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic