Re: Review Request: ClockApplet : show seconds in the tooltip

2011-01-16 Thread Marco Martin 


> On Jan. 15, 2011, 8:08 p.m., Aaron Seigo wrote:
> > i agree with Marco, that this should end up in the ClockApplet class shared 
> > by all clocks.
> > 
> > the way this probably will need to be done is to have a QObject with a 
> > dataUpdated method used for this purpose in ClockApplet that is 
> > connected/disconnected to the time engine using the current timezone on 
> > toolTipAboutToShow/Hide. then it will work with all clocks.

yeah, i like the idea.
this maybe suggests it's also time to make DataEngineConsumer public API in 
libplasma.


- Marco


---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6336/#review9635
---


On Jan. 15, 2011, 3:17 p.m., Iamluc wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6336/
> ---
> 
> (Updated Jan. 15, 2011, 3:17 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> Hi,
> 
> When using a clock applet, seconds are often not visibles. But sometimes you 
> need them.
> This patch shows them in the tooltip of the applet.
> 
> Analog-clock has been updated to refresh the tooltip every seconds.
> If this change is accepted, I could change digital-clock
> 
> Thanks !
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.cpp 1214556 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/analog-clock/clock.h 
> 1214556 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/analog-clock/clock.cpp 
> 1214556 
> 
> Diff: http://svn.reviewboard.kde.org/r/6336/diff
> 
> 
> Testing
> ---
> 
> It Works on plasmoidviewer and with a real session
> 
> 
> Thanks,
> 
> Iamluc
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Mime-type in NepomukSearchRunner

2011-01-16 Thread Amir Pakdel 

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6338/
---

Review request for Plasma, Sebastian Trueg and Vishesh Handa.


Summary
---

I have changed NepomukSearchRunner a little bit, so I could search for contents 
of notes (added by BasketNotepad) using KRunner.

It uses NIE:mimeType of the resource to set the "type" and "iconName" of the 
search result. If the resourse does not have NIE:mimeType property, 
KMimeType::findByUrl(url) is used instead (in order to find the mime-type 
associated with the url of the resource).

When running/opening the search result, preferredService of the mime-type is 
used in invokation of KRun::run; therefore, the mime-type of the resource (or 
its url) indicates the service that would be used to run the resource.


Diffs
-

  
/trunk/KDE/kdebase/workspace/plasma/generic/runners/nepomuksearch/nepomuksearchrunner.cpp
 1193800 
  
/trunk/KDE/kdebase/workspace/plasma/generic/runners/nepomuksearch/queryclientwrapper.cpp
 1193800 

Diff: http://svn.reviewboard.kde.org/r/6338/diff


Testing
---

I have searched for contents of notes that were added by BasketNotepad (using 
KRunner) and opened them in BasketNotepads.


Thanks,

Amir

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: ClockApplet : show seconds in the tooltip

2011-01-16 Thread Aaron Seigo 


> On Jan. 15, 2011, 8:08 p.m., Aaron Seigo wrote:
> > i agree with Marco, that this should end up in the ClockApplet class shared 
> > by all clocks.
> > 
> > the way this probably will need to be done is to have a QObject with a 
> > dataUpdated method used for this purpose in ClockApplet that is 
> > connected/disconnected to the time engine using the current timezone on 
> > toolTipAboutToShow/Hide. then it will work with all clocks.
> 
> Marco Martin wrote:
> yeah, i like the idea.
> this maybe suggests it's also time to make DataEngineConsumer public API 
> in libplasma.

> make DataEngineConsumer public API in libplasma

yes, it could be. taking it one step further: perhaps in libplasma2 
DataEngineManager goes private (further protecting the usage of DataEngines; 
we've seen some bugs in C++ in the past due to using DataEngineManager directly 
and incorrectly) and DataEngineConsumer becomes a public interface instead.

that said, in this case, ClockApplet can simply pass the return from 
dataEngine("time") into the QObject.


- Aaron


---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6336/#review9635
---


On Jan. 15, 2011, 3:17 p.m., Iamluc wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6336/
> ---
> 
> (Updated Jan. 15, 2011, 3:17 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> Hi,
> 
> When using a clock applet, seconds are often not visibles. But sometimes you 
> need them.
> This patch shows them in the tooltip of the applet.
> 
> Analog-clock has been updated to refresh the tooltip every seconds.
> If this change is accepted, I could change digital-clock
> 
> Thanks !
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.cpp 1214556 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/analog-clock/clock.h 
> 1214556 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/analog-clock/clock.cpp 
> 1214556 
> 
> Diff: http://svn.reviewboard.kde.org/r/6336/diff
> 
> 
> Testing
> ---
> 
> It Works on plasmoidviewer and with a real session
> 
> 
> Thanks,
> 
> Iamluc
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Mime-type in NepomukSearchRunner

2011-01-16 Thread Aaron Seigo 

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6338/#review9639
---


some code style issues, but otherwise looks good.


/trunk/KDE/kdebase/workspace/plasma/generic/runners/nepomuksearch/nepomuksearchrunner.cpp


these should go at the top of the file with the other includes. preferably 
using the CamelCase style, e.g. #include 



/trunk/KDE/kdebase/workspace/plasma/generic/runners/nepomuksearch/nepomuksearchrunner.cpp


no spaces before/after ()s



/trunk/KDE/kdebase/workspace/plasma/generic/runners/nepomuksearch/nepomuksearchrunner.cpp


whitespace



/trunk/KDE/kdebase/workspace/plasma/generic/runners/nepomuksearch/queryclientwrapper.cpp


whitespace after (s and before )s should be removed


- Aaron


On Jan. 16, 2011, 9:47 a.m., Amir Pakdel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6338/
> ---
> 
> (Updated Jan. 16, 2011, 9:47 a.m.)
> 
> 
> Review request for Plasma, Sebastian Trueg and Vishesh Handa.
> 
> 
> Summary
> ---
> 
> I have changed NepomukSearchRunner a little bit, so I could search for 
> contents of notes (added by BasketNotepad) using KRunner.
> 
> It uses NIE:mimeType of the resource to set the "type" and "iconName" of the 
> search result. If the resourse does not have NIE:mimeType property, 
> KMimeType::findByUrl(url) is used instead (in order to find the mime-type 
> associated with the url of the resource).
> 
> When running/opening the search result, preferredService of the mime-type is 
> used in invokation of KRun::run; therefore, the mime-type of the resource (or 
> its url) indicates the service that would be used to run the resource.
> 
> 
> Diffs
> -
> 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/runners/nepomuksearch/nepomuksearchrunner.cpp
>  1193800 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/runners/nepomuksearch/queryclientwrapper.cpp
>  1193800 
> 
> Diff: http://svn.reviewboard.kde.org/r/6338/diff
> 
> 
> Testing
> ---
> 
> I have searched for contents of notes that were added by BasketNotepad (using 
> KRunner) and opened them in BasketNotepads.
> 
> 
> Thanks,
> 
> Amir
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

KDE/kdelibs/plasma

2011-01-16 Thread Aaron J . Seigo 
SVN commit 1214926 by aseigo:

QString is not thread-safe; since the matches get shuttled around between 
threads, we need to protect access to it. as writing is rare, this should 
hopefully not degrade performance too much. if it passes testing, then i will 
backport this.
CCBUG:238556
CCMAIL:plasma-devel@kde.org


 M  +37 -0 querymatch.cpp  


--- trunk/KDE/kdelibs/plasma/querymatch.cpp #1214925:1214926
@@ -21,6 +21,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -38,6 +39,7 @@
 public:
 QueryMatchPrivate(AbstractRunner *r)
 : QSharedData(),
+  lock(new QReadWriteLock(QReadWriteLock::Recursive)),
   runner(r),
   type(QueryMatch::ExactMatch),
   relevance(.7),
@@ -47,6 +49,30 @@
 {
 }
 
+QueryMatchPrivate(const QueryMatchPrivate &other)
+: QSharedData(other),
+  lock(new QReadWriteLock(QReadWriteLock::Recursive))
+{
+QReadLocker lock(other.lock);
+runner = other.runner;
+type = other.type;
+relevance = other.relevance;
+selAction = other.selAction;
+enabled = other.enabled;
+idSetByData = other.idSetByData;
+id = other.id;
+text = other.text;
+subtext = other.subtext;
+icon = other.icon;
+data = other.data;
+}
+
+~QueryMatchPrivate()
+{
+delete lock;
+}
+
+QReadWriteLock *lock;
 QWeakPointer runner;
 QueryMatch::Type type;
 QString id;
@@ -116,16 +142,19 @@
 
 void QueryMatch::setText(const QString &text)
 {
+QWriteLocker locker(d->lock);
 d->text = text;
 }
 
 void QueryMatch::setSubtext(const QString &subtext)
 {
+QWriteLocker locker(d->lock);
 d->subtext = subtext;
 }
 
 void QueryMatch::setData(const QVariant & data)
 {
+QWriteLocker locker(d->lock);
 d->data = data;
 
 if (d->id.isEmpty() || d->idSetByData) {
@@ -139,6 +168,7 @@
 
 void QueryMatch::setId(const QString &id)
 {
+QWriteLocker locker(d->lock);
 if (d->runner) {
 d->id = d->runner.data()->id();
 }
@@ -152,26 +182,31 @@
 
 void QueryMatch::setIcon(const QIcon &icon)
 {
+QWriteLocker locker(d->lock);
 d->icon = icon;
 }
 
 QVariant QueryMatch::data() const
 {
+QReadLocker locker(d->lock);
 return d->data;
 }
 
 QString QueryMatch::text() const
 {
+QReadLocker locker(d->lock);
 return d->text;
 }
 
 QString QueryMatch::subtext() const
 {
+QReadLocker locker(d->lock);
 return d->subtext;
 }
 
 QIcon QueryMatch::icon() const
 {
+QReadLocker locker(d->lock);
 return d->icon;
 }
 
@@ -206,6 +241,8 @@
 return d->relevance < other.d->relevance;
 }
 
+QReadLocker locker(d->lock);
+QReadLocker otherLocker(other.d->lock);
 // when resorting to sort by alpha, we want the
 // reverse sort order!
 return d->text > other.d->text;
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel