Re: Review Request: Replace thread usage with local event loop in kio/kio/hostinfo.cpp
(I reply to the list only, as it is not related to the request itself) And no, we definitely want no nested event loop. Anything else, but not that. I agree 100%. When you see things like the same modal messagebox popping up twice (so one is visible, and another one appears called from the same line in the code) from a single-threaded application, you realize how dangerous a nested event loop can be. Andras
Re: Review Request: Replace thread usage with local event loop in kio/kio/hostinfo.cpp
On Tuesday, 9 de August de 2011 00:33:46 David Faure wrote: No no, I call wait after *terminate* ! Terminate is almost instant termination, it kills the thread. So this does not wait for the DNS lookup to be finished, it only waits for terminate to call the cleanup callback, which should be almost instant (it's just that it's in another thread). I admit that I didn't fully test the case of a very slow DNS, but quick testing on the flaky network here seemed ok, and until someone proves the contrary, I firmly believe that this does not block for the whole duration of the dns lookup. If it did, it would mean that terminate would basically do nothing ;) And no, we definitely want no nested event loop. Anything else, but not that. And anything but terminate, please. Nested event loops are better than a thread cancellation through unprotected code. If you call terminate, all bets are lost. The first time you call terminate, I will wash my hands of ANY bug you find afterwards in QtNetwork. Don't terminate the thread -- just let it exit on its own, even if it will take 30 minutes. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Re: How to create libraries in KDE Frameworks 5
Hi,
The itemmodels_export.h file is created in the build directory instead of
being checked in. That means that it must be installed with a referenece to
the build dir, like
install(FILES
${CMAKE_CURRENT_BUILD_DIR}/itemmodels_export.h
)
I think there is a mistake here to install, you need to use
CMAKE_CURRENT_BINARY_DIR.
And you have something like that:
install(FILES
${CMAKE_CURRENT_BINARY_DIR}/itemmodels_export.h
)
Regards,
Mario
Re: Review Request: Prevent KMessageBox instances with a parent from being application modal
On Aug. 8, 2011, 10:51 p.m., David Jarvie wrote: If the proposed patch is unacceptable, I suggest adding a new method static void allowWindowModal(bool allow); If called with true, this would enable the behaviour described in the apidocs for subsequent KMessageBox instances. By default, or if called with false, all KMessageBox instances would behave as now. As I said in an earlier comment, being able only to display application modal instances just isn't acceptable in some cases. What about extending the Option enum instead? - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102246/#review5525 --- On Aug. 7, 2011, 3:18 p.m., David Jarvie wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102246/ --- (Updated Aug. 7, 2011, 3:18 p.m.) Review request for kdelibs. Summary --- According to the apidocs, KMessageBox instances with a parent widget specified are supposed to be window modal, not application modal. This patch fixes this. Diffs - kdeui/dialogs/kmessagebox.cpp 939be89 Diff: http://git.reviewboard.kde.org/r/102246/diff Testing --- Tested warningYesNo(), questionYesNoCancel() with a parent widget - widgets in a different window tree were still able to be used. kmessageboxtest runs ok. Thanks, David
Re: Review Request: Add Activity Awareness to KFilePlaces* Widget (OnlyInActivity)
On May 31, 2011, 9:24 p.m., Kevin Ottens wrote: Note that I can't really comment on the activities specific parts, Ivan would probably be a better reviewer for that parts. Anyway I found a couple of smaller issues which need fixing. The below have been fixed locally, but I haven't had time to fix the bug I mentioned yet. Ivan, do you have any comments about the implementation? After exams I hope to take a look at this again. - Jeffery --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101348/#review3605 --- On June 1, 2011, 6:07 a.m., Jeffery MacEachern wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101348/ --- (Updated June 1, 2011, 6:07 a.m.) Review request for kdelibs, Kevin Ottens, Ivan Čukić, and David Faure. Summary --- Adds an Only show in this Activity option to the KFilePlaces Widget and support in the underlying model code. Currently only one activity/all activities are supported as choices; I think this should be sufficient, and anything more complicated would be hard to make usable. Diffs - kfile/CMakeLists.txt ceae140 kfile/kfileplaceeditdialog.h d5b030a kfile/kfileplaceeditdialog.cpp d798b4d kfile/kfileplacesmodel.h b3dd821 kfile/kfileplacesmodel.cpp b037084 kfile/kfileplacesview.cpp 6a343b3 Diff: http://git.reviewboard.kde.org/r/101348/diff Testing --- Tested on Project Neon/Kubuntu Natty. Created several activities, added Place bookmarks, set them to only show in the current activity, and switched activities. Everything worked as intended. EDIT: one small known issue - the OnlyInActivity setting doesn't take when the bookmark is first created; you have to hit Edit and re-check the box. Thanks, Jeffery
Re: Review Request: Prevent KMessageBox instances with a parent from being application modal
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102246/ --- (Updated Aug. 9, 2011, 8:39 a.m.) Review request for kdelibs. Changes --- The patch has been revised to use the Options parameter to specify window-modal instead, as suggested by Thomas. Summary --- According to the apidocs, KMessageBox instances with a parent widget specified are supposed to be window modal, not application modal. This patch fixes this. Diffs (updated) - kdeui/dialogs/kmessagebox.cpp 939be89 kdeui/dialogs/kmessagebox.h 286880e Diff: http://git.reviewboard.kde.org/r/102246/diff Testing --- Tested warningYesNo(), questionYesNoCancel() with a parent widget - widgets in a different window tree were still able to be used. kmessageboxtest runs ok. Thanks, David
Re: Review Request: Prevent KMessageBox instances with a parent from being application modal
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102246/#review5532
---
API looks ok. Implementation could prevent duplication by making a short
file-static function like
static void applyOptions(dialog, options)
{
[the new code]
dialog-setModal( true );
}
This will also make it easier for the next time :-)
- David
On Aug. 9, 2011, 8:39 a.m., David Jarvie wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102246/
---
(Updated Aug. 9, 2011, 8:39 a.m.)
Review request for kdelibs.
Summary
---
According to the apidocs, KMessageBox instances with a parent widget
specified are supposed to be window modal, not application modal. This patch
fixes this.
Diffs
-
kdeui/dialogs/kmessagebox.cpp 939be89
kdeui/dialogs/kmessagebox.h 286880e
Diff: http://git.reviewboard.kde.org/r/102246/diff
Testing
---
Tested warningYesNo(), questionYesNoCancel() with a parent widget - widgets
in a different window tree were still able to be used.
kmessageboxtest runs ok.
Thanks,
David
Expert advice needed on problems with oxygen + alpha channel
Hello, I'm facing a bug with oxygen and KToolBar when compositing is active that I need expert advice on. The issue: when you detach (unlock and move out of the window) a KToolbar from a main window, oxygen gives it the translucent background flag, and uses it to draw nice beveled corner on the detached toolbar. The same *was* done for Dockwidgets, and it *used to* work in both cases. Sadly enough it does not work anymore (try it with e.g. toolbars in dolphin). You get a zillion of errors message like: X Error: BadMatch (invalid parameter attributes) 8 Major opcode: 62 (X_CopyArea) Resource id: 0x22027d8 and the toolbar contents is not drawn anymore. Nor does it work for Dockwidgets in which there is a KPart (okular or kate), as was reported in bug https://bugs.kde.org/show_bug.cgi?id=273848 To fix the bug, I found a workaround that unfortunately looks nice only when using KWin. I could do the same for KToolBars, but what annoys me is - it does work without the workaround for other QDockWidgets (with no kpart in there) - it does work without the workaround for QToolBar (try, e.g. in QGit, or Quassel) - it looks ugly when using kde applications in another DE. I believe it is somehow related to XmlGui, and would rather have this fixed there, than propagating ugly workarounds. However, I have no clue where to look, how to start. Advice ? Thanks in advance, Hugo
Re: Review Request: Prevent KMessageBox instances with a parent from being application modal
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102246/#review5537 --- This review has been submitted with commit 2e2812f8e12154f2e267854c8f7c0b9d766d5233 by David Jarvie to branch KDE/4.7. - Commit On Aug. 9, 2011, 8:39 a.m., David Jarvie wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102246/ --- (Updated Aug. 9, 2011, 8:39 a.m.) Review request for kdelibs. Summary --- According to the apidocs, KMessageBox instances with a parent widget specified are supposed to be window modal, not application modal. This patch fixes this. Diffs - kdeui/dialogs/kmessagebox.cpp 939be89 kdeui/dialogs/kmessagebox.h 286880e Diff: http://git.reviewboard.kde.org/r/102246/diff Testing --- Tested warningYesNo(), questionYesNoCancel() with a parent widget - widgets in a different window tree were still able to be used. kmessageboxtest runs ok. Thanks, David
Re: Review Request: Replace thread usage with local event loop in kio/kio/hostinfo.cpp
On Monday 08 August 2011 16.28.43 Dawit A wrote:
Apologies for still not getting it.. You stated you wanted to have a
timeout to avoid a blocking UI, which as
far as I understand you would also avoid if you don't create a new
method that never blocks in the first place.
The uri filter plugins, which are primarly used to filter user input,e.g. user
typing into a konqueror's address, are time critical for theobvious reasons.
The architecture for these plugins relies on a directsynchrounous call. See
KUriFilterPlugin in kio/kio/kurifilter.h.Perhaps looking at the parent of the
plugin classes will help clarifythe problem for you. KUriFilter loads all
allowed uri filter pluginsand filters the user input by invoking
KUriFilterPlugin::filterUri.
Ok, I understand your issue;
To me there seems to be a architectural issue which you are fighting. Might be
interesting to keep this in mind for kde5.
The architectural issue I'm seeing is that there is a singleton which has a
method to filter and it blocks until the filtering is done. This is at odds
with the basic ingredient of using the network.
One is blocking and can only give a result once, but the result might get
better if we wait a bit longer. For example with DNS lookups.
Anyway; lets make do with what we have :)
Another solution for using a timeout can be something like the attached
version (I didn't even try to compile it, maybe the helper object has to be
moved to a _p.h file to get moc to run on it..)
The problem that remains with this solution is that if you have 20 plusing
that all have a timeout, your timeouts accumulate and you still get bad
performance. But I don't see a way to solve that using the current
architecture.
From 09eb6196198c057245651b7057cae9c079fbeeea Mon Sep 17 00:00:00 2001
From: Thomas Zander zan...@kde.org
Date: Tue, 9 Aug 2011 12:32:55 +0200
Subject: [PATCH 1/2] fix typos
---
kio/kio/kurifilter.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kio/kio/kurifilter.h b/kio/kio/kurifilter.h
index 289b910..c773f2d 100644
--- a/kio/kio/kurifilter.h
+++ b/kio/kio/kurifilter.h
@@ -709,7 +709,7 @@ protected:
/**
* Sets the arguments and options string in @p data to @p args if any were
- * found during filterting.
+ * found during filtering.
*/
void setArguments( KUriFilterData data, const QString args ) const;
@@ -807,7 +807,7 @@ private:
* http://kde.org;.
*
* You can also restrict the filters to be used by supplying the name of the
- * filters you want to use. By defualt all available filters are used.
+ * filters you want to use. By default all available filters are used.
*
* To use specific filters, add the names of the filters you want to use to a
* QStringList and invoke the appropriate filtering function.
--
1.7.1
From f6c8ba5f390ba214172dda8faa6d00bf95414681 Mon Sep 17 00:00:00 2001
From: Thomas Zander zan...@kde.org
Date: Tue, 9 Aug 2011 12:51:51 +0200
Subject: [PATCH 2/2] Implement the hostinfo fetch using a mutex
---
kio/kio/kurifilter.cpp | 23 ++-
1 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/kio/kio/kurifilter.cpp b/kio/kio/kurifilter.cpp
index 0144a2c..1049365 100644
--- a/kio/kio/kurifilter.cpp
+++ b/kio/kio/kurifilter.cpp
@@ -569,9 +569,30 @@ QString KUriFilterPlugin::iconNameFor(const KUrl url, KUriFilterData::UriTypes
return lookupIconNameFor(url, type);
}
+namespace {
+class CallBack : public QObject {
+Q_OBJECT
+public slots:
+void setInfo(const QHostInfo info) {
+hostInfo = info;
+mutex.unlock();
+}
+
+public:
+QHostInfo hostInfo;
+QMutex mutex;
+};
+};
+
QHostInfo KUriFilterPlugin::resolveName(const QString hostname, unsigned long timeout) const
{
-return KIO::HostInfo::lookupHost(hostname, timeout);
+CallBack cb;
+cb.mutex.lock();
+
+const int id = QHostInfo::lookupHost(hostname, cb, SLOT(setInfo(const QHostInfo)));
+cb.mutex.tryLock(timeout);
+
+return cb.hostInfo;
}
--
1.7.1
Re: playground-libs/libkvkontakte has moved to kdereview
On Tuesday 09 August 2011 12:07:49 Alexander Potashev wrote:
Hi,
playground-libs/libkvkontakte moved to kdereview today. The next
target for this project is extragear/libs. This project is a
library for interaction with the most popular social network in
Russia VKontakte.ru (also available at vk.com) using its public
API documented at
http://vkontakte.ru/pages.php?o=-1p=Документация (only in
Russian). libkvkontakte is already being used by
playground-pim/akonadi-vkontakte and the KIPI export plugin for
VKontakte which will be hopefully released with digiKam SC 2.1.0 in
September.
Some parts of code in libkvkontakte are based on
playground-pim/akonadi-facebook written by Thomas McGuire and
others.
Please, review.
Thanks.
It doesn't compile because asserts reference undeclared variables.
Attached patch shows where the errors are.
Christoph Feck (kdepepo)
KDE Quality Team
diff --git a/libkvkontakte/allnoteslistjob.cpp b/libkvkontakte/allnoteslistjob.cpp
index e095e94..59b604a 100644
--- a/libkvkontakte/allnoteslistjob.cpp
+++ b/libkvkontakte/allnoteslistjob.cpp
@@ -32,7 +32,7 @@ AllNotesListJob::AllNotesListJob(const QString accessToken, int uid)
void AllNotesListJob::startNewJob(int offset, int count)
{
-Q_ASSERT(out == 0 || out == 1);
+//Q_ASSERT(out == 0 || out == 1);
NotesListJob *job = new NotesListJob(m_accessToken, m_uid, offset, count);
connect(job, SIGNAL(result(KJob*)), this, SLOT(jobFinished(KJob*)));
@@ -67,7 +67,7 @@ void AllNotesListJob::jobFinished(KJob *kjob)
}
else {
// TODO: some new notes might have been added, what should we do then?
-Q_ASSERT(m_totalCount == listJob-totalCount());
+//Q_ASSERT(m_totalCount == listJob-totalCount());
}
// All jobs have finished
diff --git a/libkvkontakte/authenticationdialog.cpp b/libkvkontakte/authenticationdialog.cpp
index d7da524..9c95ef1 100644
--- a/libkvkontakte/authenticationdialog.cpp
+++ b/libkvkontakte/authenticationdialog.cpp
@@ -75,7 +75,7 @@ void AuthenticationDialog::setPermissions(const QStringList permissions)
void AuthenticationDialog::start()
{
-Q_ASSERT(!mAppId.isEmpty());
+Q_ASSERT(!m_appId.isEmpty());
// display= {page, popup, touch, wap}
const QString url = QString(http://api.vkontakte.ru/oauth/authorize?;
Re: Review Request: Improve Calculate/Stop buttons when folder Size is being calculated
On Aug. 9, 2011, 9:40 a.m., David Faure wrote: What I don't like about a single button, is that it creates a race. The calculation takes too long, you want to abort it. You click on the button, and half a second before, the calculation actually ended. So your click triggers a Calculate again, and the annoying calculation that you wanted to stop, is starting again. It can be stopped a second time of course, but this still makes for an annoying user experience. Thomas Lübking wrote: Simple solution: disarm (or just disable) the button for about a second after the calculation has ended, ie. clicking the button leads to no action (should be fine since recalculating after a second has no point?) Ideally the mousebuttonpress event would be eaten but the button doesn't look disabled. (click - click failed - click again) Just disconnecting from the slot for that timeframe is suboptimal, since the function might appear to be broken (click - push - nothing happens?) Setting it disabled serves that functionality but causes visual flicker and raises the question why the button was disabled. You could install an eventfilter to return true on mousepress/release events on the button and remove it from a timerevent or singleshot timer (just QTimer::singleShot() should be fine as well if you can ensure that you won't run two of them at once) Calculating... (time goes on) Calculation Done (clicking has no effect, then about 2 seconds later) Refresh - Christoph --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102149/#review5534 --- On July 30, 2011, 2:51 p.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102149/ --- (Updated July 30, 2011, 2:51 p.m.) Review request for kdelibs. Summary --- This patch improves the Calculate and Stop buttons in folder properties. Instead of having two buttons that are enabled/disabled accordingly, we only have one that toggles (technically there are still two buttons but it looks as if there was one) I removed that Calculating... label when there is already a size shown, instead the stop button says Stop Calculating and somehow serves as indicator. Also, I added a line-break after the Calculating... so the label doesn't change its size making the other elements moving around. I don't know if the additional icons (view-refresh and process-cancel) add too much clutter to the visual interface since this features is not sooo important/frequently used(?) that the buttons need an icon. Diffs - kio/kfile/kpropertiesdialog.cpp ba56f18 Diff: http://git.reviewboard.kde.org/r/102149/diff Testing --- Compiles and the buttons toggle (and react) fine. Screenshots --- Screenshot of the dialog while it is calculating http://git.reviewboard.kde.org/r/102149/s/213/ Thanks, Kai Uwe
Re: playground-libs/libkvkontakte has moved to kdereview
2011/8/9 Christoph Feck christ...@maxiom.de: It doesn't compile because asserts reference undeclared variables. Attached patch shows where the errors are. I've applied your patch to authenticationdialog.cpp and reworked error reporting in allnoteslistjob.cpp and also in allmessageslistjob.cpp. Thanks! -- Alexander Potashev
Re: Review Request: Replace thread usage with local event loop in kio/kio/hostinfo.cpp
On Aug. 8, 2011, 1:53 p.m., David Faure wrote: David Faure wrote: (Sorry, flaky wifi lost the comment) I am very much against a nested event loop (QEventLoop::exec), it's a well-known fact nowadays that it creates unexpected re-entrancy and crashes. And since I just fixed the crash (missing wait() after terminate(), see commit log), I don't think we need this change. However reusing threads might be a good idea (separate issue). Albert Astals Cid wrote: What you did seems like breaking the idea of the timeout parameter altogether since you are basically waiting (and blocking the UI) until my slow DNS answers instead of only waiting for the time the timeout specifies. Dawit Alemayehu wrote: I have the same concerns. See http://lists.kde.org/?l=kde-commitsm=131281315828663w=2. Under most circumstances, I too am against using local event loops because they are simply evil for reasons David mentioned above. However, I rather make the exception and go that route in this case to avoid constant UI blocking for those with slow DNS. David Faure wrote: No no, I call wait after *terminate* ! Terminate is almost instant termination, it kills the thread. So this does not wait for the DNS lookup to be finished, it only waits for terminate to call the cleanup callback, which should be almost instant (it's just that it's in another thread). I admit that I didn't fully test the case of a very slow DNS, but quick testing on the flaky network here seemed ok, and until someone proves the contrary, I firmly believe that this does not block for the whole duration of the dns lookup. If it did, it would mean that terminate would basically do nothing ;) And no, we definitely want no nested event loop. Anything else, but not that. I just tried a test calling that function with 0 as timeout (so it always timeouts) and sometimes it waits until 1 second so it is quite clear it is waiting for the thread. - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102238/#review5499 --- On Aug. 7, 2011, 4:07 a.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102238/ --- (Updated Aug. 7, 2011, 4:07 a.m.) Review request for kdelibs. Summary --- This patch replaces the creation of a new thread for every DNS lookup with a local even loop based solution. This change addresses the issue of crashes that can arise from terminating the thread when the desired timeout duration is exceeded. See http://git.reviewboard.kde.org/r/102179/. Diffs - kio/kio/hostinfo.cpp fcb7889 Diff: http://git.reviewboard.kde.org/r/102238/diff Testing --- Thanks, Dawit
Re: Review Request: Replace thread usage with local event loop in kio/kio/hostinfo.cpp
On Dimarts 09 Agost 2011 12:14:53 Thomas Zander wrote: On Tuesday 09 August 2011 12.59.49 Thomas Zander wrote: Another solution for using a timeout can be something like the attached version (I didn't even try to compile it, maybe the helper object has to be moved to a _p.h file to get moc to run on it..) Oh, and the mutex has to be made recursive in the constructor. Sorry, just coded this in my lunch break... This won't work since your main thread is locked in the mutex thus the signal will not be delivered since there is no event processing happening. Albert
Re: Review Request: Replace thread usage with local event loop in kio/kio/hostinfo.cpp
On Tuesday 09 August 2011 12:59:49 Thomas Zander wrote: On Monday 08 August 2011 16.28.43 Dawit A wrote: Apologies for still not getting it.. You stated you wanted to have a timeout to avoid a blocking UI, which as far as I understand you would also avoid if you don't create a new method that never blocks in the first place. The uri filter plugins, which are primarly used to filter user input,e.g. user typing into a konqueror's address, are time critical for theobvious reasons. The architecture for these plugins relies on a directsynchrounous call. See KUriFilterPlugin in kio/kio/kurifilter.h.Perhaps looking at the parent of the plugin classes will help clarifythe problem for you. KUriFilter loads all allowed uri filter pluginsand filters the user input by invoking KUriFilterPlugin::filterUri. Ok, I understand your issue; To me there seems to be a architectural issue which you are fighting. Might be interesting to keep this in mind for kde5. The architectural issue I'm seeing is that there is a singleton which has a method to filter and it blocks until the filtering is done. This is at odds with the basic ingredient of using the network. One is blocking and can only give a result once, but the result might get better if we wait a bit longer. For example with DNS lookups. Anyway; lets make do with what we have :) Another solution for using a timeout can be something like the attached version (I didn't even try to compile it, maybe the helper object has to be moved to a _p.h file to get moc to run on it..) The problem that remains with this solution is that if you have 20 plusing that all have a timeout, your timeouts accumulate and you still get bad performance. But I don't see a way to solve that using the current architecture. On the patch: By principles, Mutexes should not be hold for a long time like this. The proper thing to use here is a QWaitCondition
Re: Review Request: Replace thread usage with local event loop in kio/kio/hostinfo.cpp
I just tried a test calling that function with 0 as timeout (so it always timeouts) and sometimes it waits until 1 second so it is quite clear it is waiting for the thread. Yes, Thiago explained to me the notion of cancellation points, and proved me wrong. And since anyway terminate is a bad idea, this needs to be redesigned. Thomas: I can't see how locking a mutex can work in a single-threaded case (there are no threads in your code; you seem to rely on the fact that Qt uses a thread internally in QHostInfo? That seems dangerous...). Thiago and I came up with a design like this: * a single thread that makes async lookups. It's never blocked, it can always almost immediately read new requests from a queue and start them. * each request is encapsulated in a class that has the QString but also a QSemaphore. * the main thread can block using a timeout (QSemaphore::tryAcquire(timeout)) * the request class is stored using a shared pointer, so that it's only deleted when neither the main thread nor the worker thread need it anymore [otherwise the deletion is a problem, in the timeout case vs the normal case] -- David Faure, fa...@kde.org, http://www.davidfaure.fr Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).
Review Request: Umpteenth try to fix KCharSelect crash
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102263/ --- Review request for kdelibs and Albert Astals Cid. Summary --- This is definitively my last try to fix the KCharSelect crash. This addresses bug 235020. http://bugs.kde.org/show_bug.cgi?id=235020 Diffs - kdeui/widgets/kcharselect.cpp c511191 kdeui/widgets/kcharselect_p.h d414d23 Diff: http://git.reviewboard.kde.org/r/102263/diff Testing --- I NEVER could reproduce the crash, so I don't know if this changes anything at all. Thanks, Christoph
Re: Kamoso in extragear/multimedia
On Tue, May 24, 2011 at 6:06 PM, Aleix Pol aleix...@kde.org wrote: Hi! Alex and I, we have been working lately on the Kamoso port to QtGstreamer, which has brought us a lot of stability, until the point that we are happy enough to leave playground. After a lot of discussion about where do we thing Kamoso should go, we have decided that we want to put it in extragear/multimedia because it lets us to release whenever we are comfortable with, which we think this goes quite well with our way to work with the project. We moved kamoso yesterday to kdereview, so please take a look at what we have and, if you think there's anything that should be changed before the move, please tell us to have it addressed. Thanks! Aleix PS: Sorry for sending everywhere, but people keep telling me to send this mail in different mailing lists... Hi, Since nobody answered I'll file a bugreport to sysadmin about moving kamoso into extragear/multimedia. If anybody has any question, send me an e-mail or say hi in the DS. Aleix
Review Request: Fix missing ... in KBookmarkAction displayed text
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102262/ --- Review request for kdelibs. Summary --- When a bookmark name, is too long, its name is truncated with ... at the middle. But, QAction strip those three dots by default. This patch solves this issue by defining imageText. Diffs - kio/bookmarks/kbookmarkmenu.cc 873f4a8 Diff: http://git.reviewboard.kde.org/r/102262/diff Testing --- It works with Konqueror and rekonq. Thanks, Yoann
Re: Review Request: Fix missing ... in KBookmarkAction displayed text
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102262/#review5554 --- Since you're setting iconText, this is about actions into a toolbar, not into a menu, right? So Qt strips ... from toolbar button texts? What's the logic there? I'm just curious, the patch looks ok, from your description. - David On Aug. 9, 2011, 3:32 p.m., Yoann Laissus wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102262/ --- (Updated Aug. 9, 2011, 3:32 p.m.) Review request for kdelibs. Summary --- When a bookmark name, is too long, its name is truncated with ... at the middle. But, QAction strip those three dots by default. This patch solves this issue by defining imageText. Diffs - kio/bookmarks/kbookmarkmenu.cc 873f4a8 Diff: http://git.reviewboard.kde.org/r/102262/diff Testing --- It works with Konqueror and rekonq. Thanks, Yoann
Review Request: Speed limit in ftp kio slave
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102267/ --- Review request for kdelibs. Summary --- - This patch contains the basic code which will put the limit on download speed of the ftp data transfer. - It is looking for speed-limit meta-data for deciding how much speed control is required. - If this meta-data is not found, code will work as it was before and no speed control related code will come into picture. - This patch is the most basic one which I have testing on my system and to the extent it is controlling the speed. - Lets say if speed limit is 30 KBps then mostly will get the avg speed around 30 to 35 KBps. - I am using QTime for measuring time elapsed between two socket read call and its precision is in millisecond. Looping is taking place in microsecond and thats why I am getting almost all the time 0 as time elapsed in between two calls. - To solve the above problem usleep is introduced to make it sync with the timer. Diffs - kioslave/ftp/CMakeLists.txt e080b02 kioslave/ftp/ftp.h 0bd375b kioslave/ftp/ftp.cpp 655524a kioslave/ftp/speedController.h PRE-CREATION kioslave/ftp/speedController.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/102267/diff Testing --- Thanks, Tushar
Re: smallish project needed
On Tue, Aug 9, 2011 at 03:15, Michael Pyne mp...@kde.org wrote: On Monday, August 08, 2011 18:44:40 Tomaz Canabrava wrote: Juk is an easy target, and in need of love. Honestly I was going to recommend the same thing. I don't agree that it's (all) easy (although there is certainly a lot of low- hanging fruit), but it does have the advantage that I'm at least available by email to help guide/mentor. In addition I will be completing school very soon, which hopefully should add some time per day (although that may be offset by the new job I will be rotating to soon which will probably involve a longer commute). Either way, JuK could use some love, there's still someone mostly-active who can show interested parties around the codebase and I should have piped in on one of these requests awhile ago (but I've always assumed someone else has need the help more ;( ) Thanks guys. I've suggested him to take a look at JuK. Cheers Lydia -- Lydia Pintscher KDE Community Working Group member http://kde.org - http://about.me/lydia.pintscher
Re: Plan to transition to KDE Frameworks
On Monday 08 August 2011 00:42:50 Scott Kitterman wrote: On Saturday, August 06, 2011 09:32:02 AM David Faure wrote: .. The next step is to backport the few bits of new api that went into master and that application developers started using, into the 4.7 branch of kdelibs. I'll work on that, but help is welcome too. ... This plan seems to be contrary to the KDE Point Release Policy [1]. At this point I don't see an easy way out, but it would be good if a cutoff point for these additional API changes in 4.7 could be set (perhaps no later than 4.7.1's release). [1] http://techbase.kde.org/Policies/Minor_Point_Release_Policy Under some conditions, the policy allows new API in a stable branch. We decided that these conditions were met :-) (IIRC there were only three, and they were rather minor and safe; a KUrl::List constructor, a new exported method, a new signal) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).
Re: Review Request: Add a new KDBusService class to manage the D-Bus registration
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101902/#review5557 --- Ship it! Missing license; missing an increased d-bus timeout, looks good otherwise. Well, we worked together on this code in a train going out of Switzerland, so I'm a bit biased ;) - David On July 9, 2011, 3:43 p.m., Kevin Ottens wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101902/ --- (Updated July 9, 2011, 3:43 p.m.) Review request for kdelibs and David Faure. Summary --- This class is meant to get the D-Bus registration logic out of KApplication, so that it can also be easily provided to QCoreApplication. It also allows to easily provide the KUniqueApplication behavior, which is tested with the accompanying unit test. Note that it's a somewhat early review request, I'm aware that the APIDOX is missing for instance. It's just that it'll likely be a generally useful class, so I'd like to gather feedback early. In short, just in case you were wondering why it seems unfinished: it is unfinished. :-) Diffs - kdecore/CMakeLists.txt 4d96b50 kdecore/kernel/kdbusservice.h PRE-CREATION kdecore/kernel/kdbusservice.cpp PRE-CREATION kdecore/tests/CMakeLists.txt 7c3db77 kdecore/tests/kdbusservicetest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/101902/diff Testing --- Thanks, Kevin
Re: Plan to transition to KDE Frameworks
On Tuesday, August 09, 2011 07:05:53 PM David Faure wrote: On Monday 08 August 2011 00:42:50 Scott Kitterman wrote: On Saturday, August 06, 2011 09:32:02 AM David Faure wrote: .. The next step is to backport the few bits of new api that went into master and that application developers started using, into the 4.7 branch of kdelibs. I'll work on that, but help is welcome too. ... This plan seems to be contrary to the KDE Point Release Policy [1]. At this point I don't see an easy way out, but it would be good if a cutoff point for these additional API changes in 4.7 could be set (perhaps no later than 4.7.1's release). [1] http://techbase.kde.org/Policies/Minor_Point_Release_Policy Under some conditions, the policy allows new API in a stable branch. We decided that these conditions were met :-) (IIRC there were only three, and they were rather minor and safe; a KUrl::List constructor, a new exported method, a new signal) Just to be clear then ... No further API changes are planned for incorporation in the 4.7 branch? From a distro perspective it matters to me that the API be stable after our release (which will be with 4.7.1 or 4.7.2 depending on when they are released), so as long as it's those and no others I'm perfectly happy. Scott K
Re: Review Request: Add a new KDBusService class to manage the D-Bus registration
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101902/#review5559 --- This review has been submitted with commit d0335a61536c3e9b2dfb710cb43689e9452e488c by Kevin Ottens to branch frameworks. - Commit On July 9, 2011, 3:43 p.m., Kevin Ottens wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101902/ --- (Updated July 9, 2011, 3:43 p.m.) Review request for kdelibs and David Faure. Summary --- This class is meant to get the D-Bus registration logic out of KApplication, so that it can also be easily provided to QCoreApplication. It also allows to easily provide the KUniqueApplication behavior, which is tested with the accompanying unit test. Note that it's a somewhat early review request, I'm aware that the APIDOX is missing for instance. It's just that it'll likely be a generally useful class, so I'd like to gather feedback early. In short, just in case you were wondering why it seems unfinished: it is unfinished. :-) Diffs - kdecore/CMakeLists.txt 4d96b50 kdecore/kernel/kdbusservice.h PRE-CREATION kdecore/kernel/kdbusservice.cpp PRE-CREATION kdecore/tests/CMakeLists.txt 7c3db77 kdecore/tests/kdbusservicetest.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/101902/diff Testing --- Thanks, Kevin
Re: Review Request: Use suggested filename to determine mime-type in KParts::BrowserOpenOrSave
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102256/#review5560 --- This review has been submitted with commit b4f96a8f74431223b539da175c2ae7d0bc4322bc by Dawit Alemayehu to branch frameworks. - Commit On Aug. 8, 2011, 6:54 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102256/ --- (Updated Aug. 8, 2011, 6:54 p.m.) Review request for kdelibs and David Faure. Summary --- Use the suggested filename to determine the real mime type whenever the one supplied is the default one, i.e. application/octet-stream. This addresses bugs and 279675. http://bugs.kde.org/show_bug.cgi?id= http://bugs.kde.org/show_bug.cgi?id=279675 Diffs - kparts/browseropenorsavequestion.cpp 2ea3ca0 Diff: http://git.reviewboard.kde.org/r/102256/diff Testing --- Thanks, Dawit
