Re: Review Request 120119: KRecursiveFilterProxyModel: Fixed the model
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120119/#review74519 --- The commit log has the same tree twice. Was "D" supposed to be omitted from the second tree? Overall I'm extremely surprised that this class didn't have a unittest yet. I recently wrote many proxymodels for a customer, with full test coverage, so this hurts my eyes. I will expand this unittest once it's in. If I understand correctly, the bug in refreshAscendantMapping is that basically the code was hoping to test the "old" state of the tree, while in fact acceptRow returns the new state already (so in this case, acceptRow returns true for every ascendant, so we don't know where to stop). A cache (remembering what we have inserted into the tree) would solve this, but managing the cache would be so much more work. A more clever solution could be to store the state of things (in fact just "lastAscendant") in a slot connected to rowsAboutToBeInserted, and use that later in the slot connected to rowsInserted. What do you think of this approach? (Personally I think we need more unittests before making any kind of change :-) kdeui/itemviews/krecursivefilterproxymodel.cpp <https://git.reviewboard.kde.org/r/120119/#comment51686> refreshAll isn't used anymore, the new code always "refreshes all ascendants" kdeui/tests/krecursivefilterproxymodeltest.cpp <https://git.reviewboard.kde.org/r/120119/#comment51677> const char * (or const QString &) kdeui/tests/krecursivefilterproxymodeltest.cpp <https://git.reviewboard.kde.org/r/120119/#comment51678> Good, but don't keep that comment. In 2 years we won't know which fix this is about. kdeui/tests/krecursivefilterproxymodeltest.cpp <https://git.reviewboard.kde.org/r/120119/#comment51679> (this nested block isn't useful and even harms readability) kdeui/tests/krecursivefilterproxymodeltest.cpp <https://git.reviewboard.kde.org/r/120119/#comment51680> QCOMPARE(b, true) is always better written as QVERIFY(b), that's what QVERIFY is for. kdeui/tests/krecursivefilterproxymodeltest.cpp <https://git.reviewboard.kde.org/r/120119/#comment51682> Shouldn't rowsInserted even be emitted 3 times? Once for row1, once for child, once for subchild? kdeui/tests/krecursivefilterproxymodeltest.cpp <https://git.reviewboard.kde.org/r/120119/#comment51685> rename child2 to subchild then, it's the same setup as the previous method (-> could be factorized into a fillModel private method). This method's subchild would then become subsubchild ;) Maybe (with the next test being more complex) it would be easier to call the items 1 (row), 1.1 (first child), 1.2 (second child), 1.1.1. (subchild), 1.1.1.1 (subsubchild). Anyway, this test makes me want to add more tests, I can do that later. kdeui/tests/krecursivefilterproxymodeltest.cpp <https://git.reviewboard.kde.org/r/120119/#comment51681> QVERIFY(!...) kdeui/tests/krecursivefilterproxymodeltest.cpp <https://git.reviewboard.kde.org/r/120119/#comment51683> also check that child and subchild etc. can be found? kdeui/tests/krecursivefilterproxymodeltest.cpp <https://git.reviewboard.kde.org/r/120119/#comment51684> I would also expect 3 signal emissions here - David Faure On Sept. 10, 2014, 8:15 a.m., Christian Mollekopf wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120119/ > --- > > (Updated Sept. 10, 2014, 8:15 a.m.) > > > Review request for kdelibs and Stephen Kelly. > > > Bugs: 338950 > http://bugs.kde.org/show_bug.cgi?id=338950 > > > Repository: kdelibs > > > Description > --- > > (This problem probably applies to KF5 as well, and we'll need to forward port > this patch.) > > > KRecursiveFilterProxyModel: Fixed the model > > The model was not working properly and didn't include all items under > some circumstances. > This patch fixes the following scenarios in particular: > > * The change in sourceDataChanged is required to fix the shortcut condition. > The idea is that if the parent is already part of the model (it must be if > acceptRow returns true), > we can directly invoke dataChanged on the parent, resulting in the changed > index > getting reevaluated. However, because the recursive filterAcceptsRow version > was used > the shortcut was also used when only the current index matches t
Re: Review Request 120119: KRecursiveFilterProxyModel: Fixed the model
> On Jan. 22, 2015, 7:47 a.m., David Faure wrote:
> > kdeui/tests/krecursivefilterproxymodeltest.cpp, line 126
> > <https://git.reviewboard.kde.org/r/120119/diff/1/?file=310594#file310594line126>
> >
> > Shouldn't rowsInserted even be emitted 3 times? Once for row1, once for
> > child, once for subchild?
>
> Christian Mollekopf wrote:
> AFAIK our models typically query for children, so toplevel is enough if
> children are immediately available.
You're right. We should however check that rowsInserted was emitted at the
right "level". My usual solution for that is to come up with a string
representation for indexes. Either the data string or some 1.1.1. stuff. Makes
writing unittests easy
(QCOMPARE(rowsInsertedSignals(), QString("0,1,A")) for rowsInserted(from=0,
to=1, parent=A (index whose data is "A")).
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120119/#review74519
---
On Jan. 22, 2015, 3:11 p.m., Christian Mollekopf wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120119/
> ---
>
> (Updated Jan. 22, 2015, 3:11 p.m.)
>
>
> Review request for kdelibs and Stephen Kelly.
>
>
> Bugs: 338950
> http://bugs.kde.org/show_bug.cgi?id=338950
>
>
> Repository: kdelibs
>
>
> Description
> ---
>
> (This problem probably applies to KF5 as well, and we'll need to forward port
> this patch.)
>
>
> KRecursiveFilterProxyModel: Fixed the model
>
> The model was not working properly and didn't include all items under
> some circumstances.
> This patch fixes the following scenarios in particular:
>
> * The change in sourceDataChanged is required to fix the shortcut condition.
> The idea is that if the parent is already part of the model (it must be if
> acceptRow returns true),
> we can directly invoke dataChanged on the parent, resulting in the changed
> index
> getting reevaluated. However, because the recursive filterAcceptsRow version
> was used
> the shortcut was also used when only the current index matches the filter and
> the parent index is in fact not yet in the model. In this case we failed to
> call
> dataChanged on the right index and thus the complete branch was never added
> to the model.
>
> * The change in refreshAscendantMapping is required to include indexes that
> were
> included by descendants. The intended way how this was supposed to work is
> that we
> traverse the tree upwards and find the last index that is not yet part of the
> model.
> We would then call dataChanged on that index causing it and its descendants
> to get reevaluated.
> However, acceptRow does not reflect wether an index is already in the model
> or not.
> Consider the following model:
>
> - A
> - B
> - C
> - D
>
>
> If C is include in the model by default but D not and A & B only gets
> included due to C, we have the following model:
> - A
> - B
> - C
> - D
>
> If we then call refreshAscendantsMapping on D it will not consider B as
> already being part of the model.
> This results in the toplevel index A being considered lastAscendant, and a
> call to dataChanged on A results in
> a reevaluation of A only, which is already in the model. Thus D never gets
> added to the model.
>
> Unfortunately there is no way to probe QSortFilterProxyModel for indexes that
> are
> already part of the model. Even the const mapFromSource internally creates a
> mapping when called,
> and thus instead of revealing indexes that are not yet part of the model, it
> silently
> creates a mapping (without issuing the relevant signals!).
>
> As the only possible workaround we have to issues dataChanged for all
> ancestors
> which is ignored for indexes that are not yet mapped, and results in a
> rowsInserted
> signal for the correct indexes. It also results in superfluous dataChanged
> signals,
> since we don't know when to stop, but at least we have a properly behaving
> model
> this way.
>
>
> Diffs
> -
>
> kdeui/itemviews/krecursivefilterproxymodel.cpp
> 6d6563166bcc9637d826f577925c47d5ecbef2cd
> kdeui/tests/CMakeLists.txt f661b9177a6e0e1de7f49bc3cb9fbb5e04f427c1
> kdeui/tests/krecursivefilterproxymodeltest.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/120119/diff/
>
>
> Testing
> ---
>
>
> Thanks,
>
> Christian Mollekopf
>
>
Re: Review Request 120119: KRecursiveFilterProxyModel: Fixed the model
> On Jan. 22, 2015, 7:47 a.m., David Faure wrote: > > kdeui/itemviews/krecursivefilterproxymodel.cpp, line 149 > > <https://git.reviewboard.kde.org/r/120119/diff/1/?file=310592#file310592line149> > > > > refreshAll isn't used anymore, the new code always "refreshes all > > ascendants" > > Christian Mollekopf wrote: > The default is false which is used mostly, except on line 257 Yes but the implementation now ignores the bool, so it doesn't matter what the callers are passing ;) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120119/#review74519 --- On Jan. 22, 2015, 3:11 p.m., Christian Mollekopf wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120119/ > --- > > (Updated Jan. 22, 2015, 3:11 p.m.) > > > Review request for kdelibs and Stephen Kelly. > > > Bugs: 338950 > http://bugs.kde.org/show_bug.cgi?id=338950 > > > Repository: kdelibs > > > Description > --- > > (This problem probably applies to KF5 as well, and we'll need to forward port > this patch.) > > > KRecursiveFilterProxyModel: Fixed the model > > The model was not working properly and didn't include all items under > some circumstances. > This patch fixes the following scenarios in particular: > > * The change in sourceDataChanged is required to fix the shortcut condition. > The idea is that if the parent is already part of the model (it must be if > acceptRow returns true), > we can directly invoke dataChanged on the parent, resulting in the changed > index > getting reevaluated. However, because the recursive filterAcceptsRow version > was used > the shortcut was also used when only the current index matches the filter and > the parent index is in fact not yet in the model. In this case we failed to > call > dataChanged on the right index and thus the complete branch was never added > to the model. > > * The change in refreshAscendantMapping is required to include indexes that > were > included by descendants. The intended way how this was supposed to work is > that we > traverse the tree upwards and find the last index that is not yet part of the > model. > We would then call dataChanged on that index causing it and its descendants > to get reevaluated. > However, acceptRow does not reflect wether an index is already in the model > or not. > Consider the following model: > > - A > - B > - C > - D > > > If C is include in the model by default but D not and A & B only gets > included due to C, we have the following model: > - A > - B > - C > - D > > If we then call refreshAscendantsMapping on D it will not consider B as > already being part of the model. > This results in the toplevel index A being considered lastAscendant, and a > call to dataChanged on A results in > a reevaluation of A only, which is already in the model. Thus D never gets > added to the model. > > Unfortunately there is no way to probe QSortFilterProxyModel for indexes that > are > already part of the model. Even the const mapFromSource internally creates a > mapping when called, > and thus instead of revealing indexes that are not yet part of the model, it > silently > creates a mapping (without issuing the relevant signals!). > > As the only possible workaround we have to issues dataChanged for all > ancestors > which is ignored for indexes that are not yet mapped, and results in a > rowsInserted > signal for the correct indexes. It also results in superfluous dataChanged > signals, > since we don't know when to stop, but at least we have a properly behaving > model > this way. > > > Diffs > - > > kdeui/itemviews/krecursivefilterproxymodel.cpp > 6d6563166bcc9637d826f577925c47d5ecbef2cd > kdeui/tests/CMakeLists.txt f661b9177a6e0e1de7f49bc3cb9fbb5e04f427c1 > kdeui/tests/krecursivefilterproxymodeltest.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/120119/diff/ > > > Testing > --- > > > Thanks, > > Christian Mollekopf > >
Re: Review Request 120119: KRecursiveFilterProxyModel: Fixed the model
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120119/#review74552 --- Ship it! Looks good to me, let's get this in, I'll try to find time to improve the test (and then possibly the implementation). - David Faure On Jan. 22, 2015, 4:06 p.m., Christian Mollekopf wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120119/ > --- > > (Updated Jan. 22, 2015, 4:06 p.m.) > > > Review request for kdelibs and Stephen Kelly. > > > Bugs: 338950 > http://bugs.kde.org/show_bug.cgi?id=338950 > > > Repository: kdelibs > > > Description > --- > > (This problem probably applies to KF5 as well, and we'll need to forward port > this patch.) > > > KRecursiveFilterProxyModel: Fixed the model > > The model was not working properly and didn't include all items under > some circumstances. > This patch fixes the following scenarios in particular: > > * The change in sourceDataChanged is required to fix the shortcut condition. > The idea is that if the parent is already part of the model (it must be if > acceptRow returns true), > we can directly invoke dataChanged on the parent, resulting in the changed > index > getting reevaluated. However, because the recursive filterAcceptsRow version > was used > the shortcut was also used when only the current index matches the filter and > the parent index is in fact not yet in the model. In this case we failed to > call > dataChanged on the right index and thus the complete branch was never added > to the model. > > * The change in refreshAscendantMapping is required to include indexes that > were > included by descendants. The intended way how this was supposed to work is > that we > traverse the tree upwards and find the last index that is not yet part of the > model. > We would then call dataChanged on that index causing it and its descendants > to get reevaluated. > However, acceptRow does not reflect wether an index is already in the model > or not. > Consider the following model: > > - A > - B > - C > - D > > > If C is include in the model by default but D not and A & B only gets > included due to C, we have the following model: > - A > - B > - C > - D > > If we then call refreshAscendantsMapping on D it will not consider B as > already being part of the model. > This results in the toplevel index A being considered lastAscendant, and a > call to dataChanged on A results in > a reevaluation of A only, which is already in the model. Thus D never gets > added to the model. > > Unfortunately there is no way to probe QSortFilterProxyModel for indexes that > are > already part of the model. Even the const mapFromSource internally creates a > mapping when called, > and thus instead of revealing indexes that are not yet part of the model, it > silently > creates a mapping (without issuing the relevant signals!). > > As the only possible workaround we have to issues dataChanged for all > ancestors > which is ignored for indexes that are not yet mapped, and results in a > rowsInserted > signal for the correct indexes. It also results in superfluous dataChanged > signals, > since we don't know when to stop, but at least we have a properly behaving > model > this way. > > > Diffs > - > > kdeui/itemviews/krecursivefilterproxymodel.cpp > 6d6563166bcc9637d826f577925c47d5ecbef2cd > kdeui/tests/CMakeLists.txt f661b9177a6e0e1de7f49bc3cb9fbb5e04f427c1 > kdeui/tests/krecursivefilterproxymodeltest.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/120119/diff/ > > > Testing > --- > > > Thanks, > > Christian Mollekopf > >
Review Request 122227: KRecursiveFilterProxyModel: many many more unittests, and fixing what they found.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/17/ --- Review request for kdelibs and Christian Mollekopf. Repository: kdelibs Description --- 1) setData(false), i.e. a dataChanged that removes and item from the filter, didn't actually lead to removal. The code was only looking at changing to get in, not changing to get out. 2) On insertion, we can avoid emitting dataChanged up the chain, by finding out before the insertion which exact ancestor will be changed (lastHiddenAscendantForInsert). 3) On removal, well simplify the code (completeRemove was always true, unless ignoreRemove was set, so we only need to keep ignoreRemove), and avoid emitting dataChanged up the chain, by finding out which the last parent before one that should still be visible, and hide just that one. 4) While at it, an obvious optimization that could have been done since day one: filterAcceptsRow can return true as soon as a child wants to be shown. Diffs - kdeui/itemviews/krecursivefilterproxymodel.cpp efa286ad87ded962b20c8a581b659d1b154ebf3a kdeui/tests/krecursivefilterproxymodeltest.cpp 3bcb72980730cb22f887ae8fa5fbd91b5609aeb6 Diff: https://git.reviewboard.kde.org/r/17/diff/ Testing --- Unittest, obviously. + KMail smoke testing. Thanks, David Faure
Review Request 122252: KRecursiveFilterProxyModel: fix emitting superfluous dataChanged signals
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122252/ --- Review request for kdelibs and Christian Mollekopf. Repository: kdelibs Description --- by using an internal cache of the filtering state. The tricky part is that filterAcceptsRow() must not use the cache (too bad, it would be faster), because of the setFilterFixedString() feature which can change the filtering status of indexes without KRFPM even being informed (QSFPM has no virtual method, no event...) So it only ever writes to the cache, but when dataChanged() or row insertion/removal comes in, we can look into the cache to find the old state and compare. Diffs - kdeui/itemviews/krecursivefilterproxymodel.h c16b62186fb9203ff297bd9fd28d9c13a1c8bdc4 kdeui/itemviews/krecursivefilterproxymodel.cpp efa286ad87ded962b20c8a581b659d1b154ebf3a kdeui/tests/krecursivefilterproxymodeltest.cpp 3bcb72980730cb22f887ae8fa5fbd91b5609aeb6 Diff: https://git.reviewboard.kde.org/r/122252/diff/ Testing --- Unit tests. Using kmail (and especially testing the substring filtering in the "move dialog", which an earlier version of this patch broke). Looking at the number of calls to Akonadi::FavoriteCollectionsModel::Private::reload() for a single dataChanged() signal from the ETM, which went from 10 to 4 (still too many, but the remaining problem is elsewhere). Thanks, David Faure
Re: Review Request 122227: KRecursiveFilterProxyModel: many many more unittests, and fixing what they found.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/17/ --- (Updated Jan. 26, 2015, 8:16 a.m.) Status -- This change has been marked as submitted. Review request for kdelibs and Christian Mollekopf. Repository: kdelibs Description --- 1) setData(false), i.e. a dataChanged that removes and item from the filter, didn't actually lead to removal. The code was only looking at changing to get in, not changing to get out. 2) On insertion, we can avoid emitting dataChanged up the chain, by finding out before the insertion which exact ancestor will be changed (lastHiddenAscendantForInsert). 3) On removal, well simplify the code (completeRemove was always true, unless ignoreRemove was set, so we only need to keep ignoreRemove), and avoid emitting dataChanged up the chain, by finding out which the last parent before one that should still be visible, and hide just that one. 4) While at it, an obvious optimization that could have been done since day one: filterAcceptsRow can return true as soon as a child wants to be shown. Diffs - kdeui/itemviews/krecursivefilterproxymodel.cpp efa286ad87ded962b20c8a581b659d1b154ebf3a kdeui/tests/krecursivefilterproxymodeltest.cpp 3bcb72980730cb22f887ae8fa5fbd91b5609aeb6 Diff: https://git.reviewboard.kde.org/r/17/diff/ Testing --- Unittest, obviously. + KMail smoke testing. Thanks, David Faure
Re: Review Request 122227: KRecursiveFilterProxyModel: many many more unittests, and fixing what they found.
> On Jan. 25, 2015, 10:11 p.m., Milian Wolff wrote:
> > kdeui/tests/krecursivefilterproxymodeltest.cpp, line 26
> > <https://git.reviewboard.kde.org/r/17/diff/1/?file=344391#file344391line26>
> >
> > is this still required in qt5? this should really be added upstream,
> > imo...
I don't see this line anywhere in the Qt5 autotests, so it looks like Stephen's
metatype changes made it unnecessary. In any case, this review is for Qt4 code,
for now :)
> On Jan. 25, 2015, 10:11 p.m., Milian Wolff wrote:
> > kdeui/tests/krecursivefilterproxymodeltest.cpp, line 90
> > <https://git.reviewboard.kde.org/r/17/diff/1/?file=344391#file344391line90>
> >
> > style: { on newline?
fixed
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/17/#review74725
-------
On Jan. 23, 2015, 6:11 p.m., David Faure wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/17/
> ---
>
> (Updated Jan. 23, 2015, 6:11 p.m.)
>
>
> Review request for kdelibs and Christian Mollekopf.
>
>
> Repository: kdelibs
>
>
> Description
> ---
>
> 1) setData(false), i.e. a dataChanged that removes and item from the filter,
> didn't actually lead to removal. The code was only looking at changing to
> get in, not changing to get out.
>
> 2) On insertion, we can avoid emitting dataChanged up the chain, by
> finding out before the insertion which exact ancestor will be changed
> (lastHiddenAscendantForInsert).
>
> 3) On removal, well simplify the code (completeRemove was always true, unless
> ignoreRemove was set, so we only need to keep ignoreRemove), and avoid
> emitting dataChanged up the chain, by finding out which the last parent
> before one that should still be visible, and hide just that one.
>
> 4) While at it, an obvious optimization that could have been done
> since day one: filterAcceptsRow can return true as soon as a child wants
> to be shown.
>
>
> Diffs
> -
>
> kdeui/itemviews/krecursivefilterproxymodel.cpp
> efa286ad87ded962b20c8a581b659d1b154ebf3a
> kdeui/tests/krecursivefilterproxymodeltest.cpp
> 3bcb72980730cb22f887ae8fa5fbd91b5609aeb6
>
> Diff: https://git.reviewboard.kde.org/r/17/diff/
>
>
> Testing
> ---
>
> Unittest, obviously.
> + KMail smoke testing.
>
>
> Thanks,
>
> David Faure
>
>
Re: Review Request 122252: KRecursiveFilterProxyModel: fix emitting superfluous dataChanged signals
> On Jan. 26, 2015, 9:41 a.m., Christian Mollekopf wrote: > > Looks reasonable to me. I'll apply the patch locally and test it for a > > while. > > Christian Mollekopf wrote: > This patch brings the original problem back, that shared folders do not > appear until something causes a dataChanged signal (usually a sync). Since > the model now seems to be behaving correctly, I assume the kmail model stack > is buggy in yet another place (would have been to trivial otherwise wouldn't > it?), and the superfluous dataChanged signal just happened to hide that > problem. > > Christian Mollekopf wrote: > I tracked the problem down to still be in FolderTreeWidgetProxyModel > (which is a KRecursiveFilterProxyModel). I know the relevant index makes it > through the sourcemodel because of the debug output added in > FolderTreeWidgetProxyModel::acceptRow (which also returns the correct > values). I'm fairly sure that the model is the cause for the folder not > showing up because I set the foldertreewidgetproxymodel directly as source of > the folderTreeView (a QTreeView). Given that the filtering seems to work > correctly, and assuming QTreeView isn't buggy, the reason for the folder not > showing up has to be in KRecursiveFilterProxyModel (right?). The problem is > most likely timing/order dependant because I cannot reproduce the problem > with another user but the same kmail/kdelibs version + the same account. > > The problematic folder tree looks as follows: > ``` > * Shared Folders (no mimetype) > ** shared (no mimetype) > *** lists (no mimetype) > kde.org (no mimetype) > * pim (mail mimetype) > * ... > ... > *** ... > ``` > > The problem is that the complete folder hierarchy, including "Shared > Folders" doesn't make it into the visible tree. The top 4 folders of the > hierarch would of course only be pulled in by the actual mail folder (pim). > > So far I couldn't find the actual problem to write another testcase, but > I have to assume that KRecursiveFilterProxyModel ist still buggy, and the > additional dataChanged signal just happen to rectify the problem. This definitely sounds like missing rowsInserted signals from the proxy. I'll try adding a testcase where the source model does rowsInserted(1), rowsInserted(1.1), rowsInserted(1.1.1 + with flag set). - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122252/#review74753 --- On Jan. 25, 2015, 6:51 p.m., David Faure wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122252/ > --- > > (Updated Jan. 25, 2015, 6:51 p.m.) > > > Review request for kdelibs and Christian Mollekopf. > > > Repository: kdelibs > > > Description > --- > > by using an internal cache of the filtering state. > > The tricky part is that filterAcceptsRow() must not use the cache > (too bad, it would be faster), because of the setFilterFixedString() > feature which can change the filtering status of indexes without > KRFPM even being informed (QSFPM has no virtual method, no event...) > So it only ever writes to the cache, but when dataChanged() > or row insertion/removal comes in, we can look into the cache > to find the old state and compare. > > > Diffs > - > > kdeui/itemviews/krecursivefilterproxymodel.h > c16b62186fb9203ff297bd9fd28d9c13a1c8bdc4 > kdeui/itemviews/krecursivefilterproxymodel.cpp > efa286ad87ded962b20c8a581b659d1b154ebf3a > kdeui/tests/krecursivefilterproxymodeltest.cpp > 3bcb72980730cb22f887ae8fa5fbd91b5609aeb6 > > Diff: https://git.reviewboard.kde.org/r/122252/diff/ > > > Testing > --- > > Unit tests. > > Using kmail (and especially testing the substring filtering in the "move > dialog", which an earlier version of this patch broke). > Looking at the number of calls to > Akonadi::FavoriteCollectionsModel::Private::reload() for a single > dataChanged() signal from the ETM, which went from 10 to 4 (still too many, > but the remaining problem is elsewhere). > > > Thanks, > > David Faure > >
Re: Review Request 122252: KRecursiveFilterProxyModel: fix emitting superfluous dataChanged signals
> On Jan. 26, 2015, 9:41 a.m., Christian Mollekopf wrote:
> > Looks reasonable to me. I'll apply the patch locally and test it for a
> > while.
>
> Christian Mollekopf wrote:
> This patch brings the original problem back, that shared folders do not
> appear until something causes a dataChanged signal (usually a sync). Since
> the model now seems to be behaving correctly, I assume the kmail model stack
> is buggy in yet another place (would have been to trivial otherwise wouldn't
> it?), and the superfluous dataChanged signal just happened to hide that
> problem.
>
> Christian Mollekopf wrote:
> I tracked the problem down to still be in FolderTreeWidgetProxyModel
> (which is a KRecursiveFilterProxyModel). I know the relevant index makes it
> through the sourcemodel because of the debug output added in
> FolderTreeWidgetProxyModel::acceptRow (which also returns the correct
> values). I'm fairly sure that the model is the cause for the folder not
> showing up because I set the foldertreewidgetproxymodel directly as source of
> the folderTreeView (a QTreeView). Given that the filtering seems to work
> correctly, and assuming QTreeView isn't buggy, the reason for the folder not
> showing up has to be in KRecursiveFilterProxyModel (right?). The problem is
> most likely timing/order dependant because I cannot reproduce the problem
> with another user but the same kmail/kdelibs version + the same account.
>
> The problematic folder tree looks as follows:
> ```
> * Shared Folders (no mimetype)
> ** shared (no mimetype)
> *** lists (no mimetype)
> kde.org (no mimetype)
> * pim (mail mimetype)
> * ...
> ...
> *** ...
> ```
>
> The problem is that the complete folder hierarchy, including "Shared
> Folders" doesn't make it into the visible tree. The top 4 folders of the
> hierarch would of course only be pulled in by the actual mail folder (pim).
>
> So far I couldn't find the actual problem to write another testcase, but
> I have to assume that KRecursiveFilterProxyModel ist still buggy, and the
> additional dataChanged signal just happen to rectify the problem.
>
> David Faure wrote:
> This definitely sounds like missing rowsInserted signals from the proxy.
>
> I'll try adding a testcase where the source model does rowsInserted(1),
> rowsInserted(1.1), rowsInserted(1.1.1 + with flag set).
Hmm, that's exactly what testInsert() already does.
If you can't reproduce this on another user, I guess there's little point in me
trying to reproduce it.
Can you try to trace:
- the rows{AboutToBe,}Inserted signals emitted by the source
- the rowsInserted and dataChanged emitted by the proxy
?
The first information will give us more chances of writing a testcase that
reproduces the issue,
and (assuming the first information looks correct), the second information will
confirm that it's the proxy being buggy, not something else.
If there's a way to avoid too much unrelated noise from other folders, you
could also uncomment all qDebugs in krfpm.cpp, so we get a full trace of what
happens internally.
All this with this patch applied, of course.
- David
-------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122252/#review74753
---
On Jan. 25, 2015, 6:51 p.m., David Faure wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122252/
> ---
>
> (Updated Jan. 25, 2015, 6:51 p.m.)
>
>
> Review request for kdelibs and Christian Mollekopf.
>
>
> Repository: kdelibs
>
>
> Description
> ---
>
> by using an internal cache of the filtering state.
>
> The tricky part is that filterAcceptsRow() must not use the cache
> (too bad, it would be faster), because of the setFilterFixedString()
> feature which can change the filtering status of indexes without
> KRFPM even being informed (QSFPM has no virtual method, no event...)
> So it only ever writes to the cache, but when dataChanged()
> or row insertion/removal comes in, we can look into the cache
> to find the old state and compare.
>
>
> Diffs
> -
>
> kdeui/itemviews/krecursivefilterproxymodel.h
> c16b62186fb9203ff297bd9fd28d9c13a1c8bdc4
> kdeui/itemviews/krecursivefilterproxymodel.cpp
> efa286ad87ded962b20c8a581b659d1b154ebf3a
> kdeui/tests/krecursivefil
Re: Review Request 122341: Port Thumbnail KIO Slave Away from KDELibs4Support
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122341/#review75090 --- thumbnail/thumbnail.cpp <https://git.reviewboard.kde.org/r/122341/#comment51967> Why this change? It's very wrong. The first arg is a mimetype, therefore you need KMimeTypeTrader. It compiles with KServiceTypeTrader by stuffing the mimetype into "const QString& serviceType" and by stuffing the servicetype into "const QString &constraint"... thumbnail/thumbnail.cpp <https://git.reviewboard.kde.org/r/122341/#comment51965> This (which predates this patch) is wrong, it should be QUrl::fromLocalFile(filePath). And then I would rename this variable to fileUrl or something, to make it clearer that it's a URL. thumbnail/thumbnail.cpp <https://git.reviewboard.kde.org/r/122341/#comment51966> This will be in the current dir, it should probably be QDir::tempPath() + "/kde-" instead. - David Faure On Jan. 31, 2015, 11:07 p.m., David Narváez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122341/ > --- > > (Updated Jan. 31, 2015, 11:07 p.m.) > > > Review request for kde-workspace, Bhushan Shah and David Faure. > > > Repository: kio-extras > > > Description > --- > > Only major difference would be the lack of fallback to KFMI. Maybe we could > implement thumbnail features in KFileMetadata? > > > Diffs > - > > thumbnail/thumbnail.cpp 39e8de5 > > Diff: https://git.reviewboard.kde.org/r/122341/diff/ > > > Testing > --- > > Only tested compilation. > > > Thanks, > > David Narváez > >
Re: Review Request 122330: Associate *.qmltypes and *.qmlproject files with the text/x-qml mime type
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122330/#review75179 --- -1 for not submitting this upstream for shared-mime-info. I took care of it, it's committed, will be in s-m-i 1.4. - David Faure On Feb. 1, 2015, 7:28 p.m., Denis Steckelmacher wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122330/ > --- > > (Updated Feb. 1, 2015, 7:28 p.m.) > > > Review request for KDE Frameworks, kdelibs and KDevelop. > > > Repository: kcoreaddons > > > Description > --- > > The KDevelop QML/JS plugin sometimes needs to parse .qmltypes files (in order > to list the content of installed QML modules, for instance), but KDevelop > requires that files parsed using the QML/JS plugin have the text/x-qml mime > type. > > Because .qmltypes and .qmlproject files are valid QML files (they follow the > standard QML syntax), this patch proposes to add these extensions to the ones > associated with the text/x-qml mime type. > > > Diffs > - > > src/mimetypes/kde5.xml cc9f71e > > Diff: https://git.reviewboard.kde.org/r/122330/diff/ > > > Testing > --- > > Installing kcoreaddons and updating the system MIME database allows KDevelop > to detect that files with the .qmltypes and .qmlproject extensions have to be > parsed using the QML/JS language support plugin. This allows QML files to use > QML modules installed system-wide (and fixes the unit tests of kdev-qmljs). > > > Thanks, > > Denis Steckelmacher > >
Re: Review Request 122341: Port Thumbnail KIO Slave Away from KDELibs4Support
> On fév. 1, 2015, 2:40 après-midi, David Faure wrote: > > thumbnail/thumbnail.cpp, line 384 > > <https://git.reviewboard.kde.org/r/122341/diff/1/?file=345985#file345985line384> > > > > Why this change? It's very wrong. > > > > The first arg is a mimetype, therefore you need KMimeTypeTrader. > > > > It compiles with KServiceTypeTrader by stuffing the mimetype into > > "const QString& serviceType" and by stuffing the servicetype into "const > > QString &constraint"... > > David Narváez wrote: > So how do you do this in KF5? err, exactly the same as in kdelibs4... - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122341/#review75090 --- On jan. 31, 2015, 11:07 après-midi, David Narváez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122341/ > --- > > (Updated jan. 31, 2015, 11:07 après-midi) > > > Review request for kde-workspace, Bhushan Shah and David Faure. > > > Repository: kio-extras > > > Description > --- > > Only major difference would be the lack of fallback to KFMI. Maybe we could > implement thumbnail features in KFileMetadata? > > > Diffs > - > > thumbnail/thumbnail.cpp 39e8de5 > > Diff: https://git.reviewboard.kde.org/r/122341/diff/ > > > Testing > --- > > Only tested compilation. > > > Thanks, > > David Narváez > >
Re: Review Request 122394: Fix OSX library names in kdeinit5.app
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122394/#review75253 --- Ship it! René, these are shared libs, not plugins, so their name is correct. - David Faure On fév. 2, 2015, 8:51 après-midi, Jeremy Whiting wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122394/ > --- > > (Updated fév. 2, 2015, 8:51 après-midi) > > > Review request for KDE Software on Mac OS X, kdelibs, David Faure, and Ian > Wadham. > > > Bugs: 343707 > https://bugs.kde.org/show_bug.cgi?id=343707 > > > Repository: kinit > > > Description > --- > > OSX Doesn't have .so libraries, so use OSX names in kdeinit5.app to load the > correct libraries needed. > > > Diffs > - > > src/kdeinit/kinit.cpp 3c3c913 > > Diff: https://git.reviewboard.kde.org/r/122394/diff/ > > > Testing > --- > > kdeinit5.app no longer complains about the missing .so libraries. > > > Thanks, > > Jeremy Whiting > >
Re: Review Request 122341: Port Thumbnail KIO Slave Away from KDELibs4Support
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122341/#review75469 --- thumbnail/thumbnail.cpp <https://git.reviewboard.kde.org/r/122341/#comment52188> there can be one issue here, if the destination already exists (but couldn't be loaded for some reason, e.g. corrupt file). KDE::rename would overwrite an existing dest, while QFile::rename doesn't do that. The proper way to implement "save to temp file and then move over existing destination" is to use QSaveFile. It's a corner case so I'm ok with it being fixed in a later commit. Maybe in this one a QFile::remove on the dest is enough, with a comment "// not atomic! should use QSaveFile" - David Faure On Feb. 5, 2015, 8:59 a.m., David Narváez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122341/ > --- > > (Updated Feb. 5, 2015, 8:59 a.m.) > > > Review request for kde-workspace, Bhushan Shah and David Faure. > > > Repository: kio-extras > > > Description > --- > > Only major difference would be the lack of fallback to KFMI. Maybe we could > implement thumbnail features in KFileMetadata? > > > Diffs > - > > thumbnail/thumbnail.cpp 39e8de5 > > Diff: https://git.reviewboard.kde.org/r/122341/diff/ > > > Testing > --- > > Only tested compilation. > > > Thanks, > > David Narváez > >
Re: Review Request 122341: Port Thumbnail KIO Slave Away from KDELibs4Support
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122341/#review75567 --- Ship it! Nice :) - David Faure On Feb. 6, 2015, 3:50 p.m., David Narváez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122341/ > --- > > (Updated Feb. 6, 2015, 3:50 p.m.) > > > Review request for kde-workspace, Bhushan Shah and David Faure. > > > Repository: kio-extras > > > Description > --- > > Only major difference would be the lack of fallback to KFMI. Maybe we could > implement thumbnail features in KFileMetadata? > > > Diffs > - > > thumbnail/thumbnail.cpp 39e8de5 > > Diff: https://git.reviewboard.kde.org/r/122341/diff/ > > > Testing > --- > > Only tested compilation. > > > Thanks, > > David Narváez > >
Re: Review Request 122471: Enable Compilation of about Protocol
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122471/#review75586 --- What Laurent said, plus a bug in the commit log: you ported away from KUrl, not QUrl :-) - David Faure On Feb. 7, 2015, 10:23 p.m., David Narváez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122471/ > --- > > (Updated Feb. 7, 2015, 10:23 p.m.) > > > Review request for kde-workspace and David Faure. > > > Repository: kio-extras > > > Description > --- > > Basically porting away from QUrl and KDE_EXPORT > > > Diffs > - > > CMakeLists.txt 3379ce7 > about/kio_about.h 620d6aa > about/kio_about.cpp d7396d8 > > Diff: https://git.reviewboard.kde.org/r/122471/diff/ > > > Testing > --- > > Tested compilation and installation: > > $ find /home/david/kio-install/ -name "*about*" > /home/david/kio-install/lib64/plugins/kio_about.so > > /home/david/kio-install/share/kservices5/about.protocol > > > Thanks, > > David Narváez > >
Re: Review Request 122471: Enable Compilation of about Protocol
> On Feb. 8, 2015, 9:19 a.m., Laurent Montel wrote:
> > about/kio_about.cpp, line 59
> > <https://git.reviewboard.kde.org/r/122471/diff/1/?file=347423#file347423line59>
> >
> > QApplication app(argc, argv);
> > app.setApplicationName(QLatin1String("kio_about"));
> >
> > => you remove KComponentData which is in kdelibs4support
Well, QCoreApplication is enough.
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122471/#review75584
---
On Feb. 7, 2015, 10:23 p.m., David Narváez wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122471/
> ---
>
> (Updated Feb. 7, 2015, 10:23 p.m.)
>
>
> Review request for kde-workspace and David Faure.
>
>
> Repository: kio-extras
>
>
> Description
> ---
>
> Basically porting away from QUrl and KDE_EXPORT
>
>
> Diffs
> -
>
> CMakeLists.txt 3379ce7
> about/kio_about.h 620d6aa
> about/kio_about.cpp d7396d8
>
> Diff: https://git.reviewboard.kde.org/r/122471/diff/
>
>
> Testing
> ---
>
> Tested compilation and installation:
>
> $ find /home/david/kio-install/ -name "*about*"
> /home/david/kio-install/lib64/plugins/kio_about.so
>
> /home/david/kio-install/share/kservices5/about.protocol
>
>
> Thanks,
>
> David Narváez
>
>
Re: Review Request 122330: Associate *.qmltypes and *.qmlproject files with the text/x-qml mime type
> On Feb. 1, 2015, 8:32 p.m., David Faure wrote: > > -1 for not submitting this upstream for shared-mime-info. > > > > I took care of it, it's committed, will be in s-m-i 1.4. > > Milian Wolff wrote: > Could we add a comment to this file then, telling people to work on > upstream smi instead then? Done. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122330/#review75179 --- On Feb. 1, 2015, 7:28 p.m., Denis Steckelmacher wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122330/ > --- > > (Updated Feb. 1, 2015, 7:28 p.m.) > > > Review request for KDE Frameworks, kdelibs and KDevelop. > > > Repository: kcoreaddons > > > Description > --- > > The KDevelop QML/JS plugin sometimes needs to parse .qmltypes files (in order > to list the content of installed QML modules, for instance), but KDevelop > requires that files parsed using the QML/JS plugin have the text/x-qml mime > type. > > Because .qmltypes and .qmlproject files are valid QML files (they follow the > standard QML syntax), this patch proposes to add these extensions to the ones > associated with the text/x-qml mime type. > > > Diffs > - > > src/mimetypes/kde5.xml cc9f71e > > Diff: https://git.reviewboard.kde.org/r/122330/diff/ > > > Testing > --- > > Installing kcoreaddons and updating the system MIME database allows KDevelop > to detect that files with the .qmltypes and .qmlproject extensions have to be > parsed using the QML/JS language support plugin. This allows QML files to use > QML modules installed system-wide (and fixes the unit tests of kdev-qmljs). > > > Thanks, > > Denis Steckelmacher > >
KDE Frameworks 5.7.0 released
14th February 2015. KDE today announces the release of KDE Frameworks 5.7.0. KDE Frameworks are 60 addon libraries to Qt which provide a wide variety of commonly needed functionality in mature, peer reviewed and well tested libraries with friendly licensing terms. For an introduction see the Frameworks 5.0 release announcement. General A number of fixes for compiling with the upcoming Qt 5.5 KActivities Fixed starting and stopping activities Fixed activity preview showing wrong wallpaper occasionally KArchive Create temporary files in the temp dir rather than in the current directory KAuth Fixed generation of KAuth DBus helper service files KCMUtils Fixed assert when dbus paths contain a '.' KCodecs Added support for CP949 to KCharsets KConfig kconf_update no longer processes *.upd file from KDE SC 4. Add "Version=5" to top of the upd file for updates that should be applied to Qt5/KF5 applications Fixed KCoreConfigSkeleton when toggling a value with saves in between Fixed using KSharedConfig in global object destructor KConfigWidgets KRecentFilesAction: fixed menu entry order (so it matches the kdelibs4 order) KCoreAddons KAboutData: Call addHelpOption and addVersionOption automatically, for convenience and consistency KAboutData: Bring back "Please use http://bugs.kde.org to report bugs." when no other email/url is set KAutoSaveFile: allStaleFiles() now works as expected for local files, fixed staleFiles() too KRandomSequence now uses int's internally and exposes int-api for 64-bit unambiguity Mimetype definitions: *.qmltypes and *.qmlproject files also have the text/x-qml mime type KShell: make quoteArgs quote urls with QChar::isSpace(), unusual space characters were not handled properly KSharedDataCache: fix creation of directory containing the cache (porting bug) KDBusAddons Added helper method KDEDModule::moduleForMessage for writing more kded-like daemons, such as kiod KDeclarative Added a plotter component Added overload method for Formats::formatDuration taking int New properties paintedWidth and paintedHeight added to QPixmapItem and QImageItem Fixed painting QImageItem and QPixmapItem Kded Add support for loading kded modules with JSON metadata KGlobalAccel Now includes the runtime component, making this a tier3 framework Made the Windows backend work again Re-enabled the Mac backend Fixed crash in KGlobalAccel X11 runtime shutdown KI18n Mark results as required to warn when API is misused Added BUILD_WITH_QTSCRIPT buildsystem option to allow a reduced feature-set on embedded systems KInit OSX: load the correct shared libraries at runtime Mingw compilation fixes KIO Fixed crash in jobs when linking to KIOWidgets but only using a QCoreApplication Fixed editing web shortcuts Added option KIOCORE_ONLY, to compile only KIOCore and its helper programs, but not KIOWidgets or KIOFileWidgets, thus reducing greatly the necessary dependencies Added class KFileCopyToMenu, which adds Copy To / Move To" to popupmenus SSL-enabled protocols: added support for TLSv1.1 and TLSv1.2 protocols, remove SSLv3 Fixed negotiatedSslVersion and negotiatedSslVersionName to return the actual negotiated protocol Apply the entered URL to the view when clicking the button that switches the URL navigator back to breadcrumb mode Fixed two progress bars/dialogs appearing for copy/move jobs KIO now uses its own daemon, kiod, for out-of-process services previously running in kded, in order to reduce dependencies; currently only replaces kssld Fixed "Could not write to " error when kioexec is triggered Fixed "QFileInfo::absolutePath: Constructed with empty filename" warnings when using KFilePlacesModel KItemModels Fixed KRecursiveFilterProxyModel for Qt 5.5.0+, due to QSortFilterProxyModel now using the roles parameter to the dataChanged signal KNewStuff Always reload xml data from remote urls KNotifications Documentation: mentionned the file name requirements of .notifyrc files Fixed dangling pointer to KNotification Fixed leak of knotifyconfig Install missing knotifyconfig header KPackage Renamed kpackagetool man to kpackagetool5 Fixed installation on case-insensitive filesystems Kross Fixed Kross::MetaFunction so it works with Qt5's metaobject system KService Include unknown properties when converting KPluginInfo from KService KPluginInfo: fixed properties not being copied from KService::Ptr OS X: performance fix for kbuildsycoca4 (skip app bundles) KTextEditor Fixed high-precision touchpad scrolling Do not emit documentUrlChanged during reload Do not break cursor position on document reload in lines with tabs Do not re(un)fold the first line if it was manually (un)folded vimode: command history through arrow keys Do not try to create a digest when we get a KDirWatch::deleted() signal Performance: remove global initializations KUnitConversi
Re: kio-extras
On Tuesday 24 February 2015 19:56:15 Albert Astals Cid wrote: > El Dijous, 22 de gener de 2015, a les 10:42:07, Jonathan Riddell va escriure: > > kio-extras is currently released part of Plasma 5. It's need said > > that it would be better to be part of applications because they're > > plugins used by applications and typically not by the desktop. With > > Plasma 5.2 about to go out shall I ask for kio-extras to be moved and > > if so into which module? > > Why does even such a module as kio-extras exist, isn't the purpose of > frameworks effort that frameworks are containerized, i.e. you get all you > need just by downloading the framework? Well the question is whether "all you need" includes 50 kioslaves. It depends on the need. Not all KIO users need 50 kioslaves. In fact when I deployed recently for a company, they didn't even need half of the ones that are already in kio. I think kio-extras makes sense (especially since it brings other dependencies, not just "more code"). I just think it should be either a framework or treated like an app, but definitely not a workspace thing. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KDE Telepathy has an unreleased dependency
On Wednesday 25 February 2015 02:04:58 Albert Astals Cid wrote: > El Dimarts, 24 de febrer de 2015, a les 11:52:55, Martin Klapetek va escriure: > > I just realized that one of the main dependencies, KPeople, will be > > released only after KDE Applications Beta 2 as it's targeting KF 5.8 > > (released 12th March > > while KDE Apps Beta 2 is released 11th March). > > > > I'm not sure what should be done in this case; we could perhaps move the > > 5.8 release two days earlier, but this would still miss Beta 1. > > > > So uhh...what should we do? > > I can see three options: > a) Do not release KPeople as part of frameworks but as part of KDE > Applications 15.04 (and move it later to frameworks) > b) Do not release KDE Telepathy as part of KDE Applications 15.04 > c) Give KDE Telepathy an exception to only join on Beta 3 of KDE > Applications 15.04 I see a fourth option: d) make a KPeople 5.7 release NOW If it's ready, I can make that happen with two commands or so. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KDE Telepathy has an unreleased dependency
On Wednesday 25 February 2015 15:38:19 Aleix Pol wrote: > At the moment KPeople is an optional dependency, there's still the > possibility to use it without KPeople. I guess you mean s/KPeople/Baloo/g here. The problem is: Baloo depends on a few frameworks, and now we would have KPeople depending on Baloo. So it wouldn't be possible to build all of frameworks together, one would need to interject Baloo in the middle of it, in order to be able to build "with all options" i.e. with this optional dependency. The reason "it's optional so it doesn't matter" means you might as well delete the code, since it becomes impractical to build it anyway. More seriously I would suggest moving the plugin to Baloo maybe, if it makes sense for it to depend on KPeople, or moving it in workspace maybe (but then it's not available in other workspaces). Or to this new "product" (set of modules) that I think we should have to host drkonqi, kio-extras etc., i.e. the set of runtime deps common to workspace and applications. This issue keeps popping up. If you want a short term solution, I'd say the plugin should stay in playground somewhere. I don't like the optional dependency thing because it will still show up in dependency diagrams, making a mess with baloo being in the middle of frameworks while it was decided that it's not. > This shouldn't be a show-stopper, even less for the Beta versions. KF 5.x isn't a beta. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: kio-extras
On Thursday 26 February 2015 01:02:04 Albert Astals Cid wrote: > Really it has to be either part of KIO or another framework, i mean, if > you're using the desktop only (and no apps) you're also going to need it. Can you expand on why "you are going to need it"? I install workspace only. OK, I have a workspace. It works, but it has limited functionality obviously. No calculator -> I install kcalc No text editor -> I install kwrite No file manager -> I install dolphin No support for sftp or thumbnails in dolphin -> I install kio-extras. (of course this is written as a compiling-from-scratch type setup, in practice the binary package for dolphin would bring it in as a dependency) Ah. Is this about kio_desktop? That one should *definitely* be in workspace. But it doesn't have to be in kio-extras. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
KDE Frameworks 5.8 released
New frameworks: * KPeople, provides access to all contacts and the people who hold them * KXmlRpcClient, interaction with XMLRPC services ### General * A number of build fixes for compiling with the upcoming Qt 5.5 ### KActivities * Resources scoring service is now finalized ### KArchive * Stop failing on ZIP files with redundant data descriptors ### KCMUtils * Restore KCModule::setAuthAction ### KCoreAddons * KPluginMetadata: add support for Hidden key ### KDeclarative * Prefer exposing lists to QML with QJsonArray * Handle non default devicePixelRatios in images * Expose hasUrls in DeclarativeMimeData * Allow users to configure how many horizontal lines are drawn ### KDocTools * Fix the build on MacOSX when using Homebrew * Better styling of media objects (images, ...) in documentation * Encode invalid chars in paths used in XML DTDs, avoiding errors ### KGlobalAccel * Activation timestamp set as dynamic property on triggered QAction. ### KIconThemes * Fix QIcon::fromTheme(xxx, someFallback) would not return the fallback ### KImageFormats * Make PSD image reader endianess-agnostic. ### KIO * Deprecate UDSEntry::listFields and add the UDSEntry::fields method which returns a QVector without costly conversion. * Sync bookmarkmanager only if change was by this process (bug 343735) * Fix startup of kssld5 dbus service * Implement quota-used-bytes and quota-available-bytes from RFC 4331 to enable free space information in http ioslave. ### KNotifications * Delay the audio init until actually needed * Fix notification config not applying instantly * Fix audio notifications stopping after first file played ### KNotifyConfig * Add optional dependency on QtSpeech to reenable speaking notifications. ### KService * KPluginInfo: support stringlists as properties ### KTextEditor * Add word count statistics in statusbar * vimode: fix crash when removing last line in Visual Line mode ### KWidgetsAddons * Make KRatingWidget cope with devicePixelRatio ### KWindowSystem * KSelectionWatcher and KSelectionOwner can be used without depending on QX11Info. * KXMessages can be used without depending on QX11Info ### NetworkManagerQt * Add new properties and methods from NetworkManager 1.0.0 Plasma framework * Fix plasmapkg2 for translated systems * Improve tooltip layout * Make it possible to let plasmoids to load scripts outside the plasma package ... ### Buildsystem changes (extra-cmake-modules) * Extend ecm_generate_headers macro to also support CamelCase.h headers -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 122652: Use correct default value when UDS_ACCESS/UDS_FILE_TYPE is not set
> On March 17, 2015, 4:38 p.m., David Faure wrote: > > Ship It! Yes, both in kdelibs 4 and in the kio framework. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122652/#review77642 --- On March 17, 2015, 4:17 p.m., Stefan Brüns wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122652/ > --- > > (Updated March 17, 2015, 4:17 p.m.) > > > Review request for KDE Frameworks, kdelibs and David Faure. > > > Bugs: 339193 > http://bugs.kde.org/show_bug.cgi?id=339193 > > > Repository: kdelibs > > > Description > --- > > The default value for UDSEntry::numberValue(...) is 0, whereas KFileItem > uses the special value KFileItem::Unknown == (mode_t) -1. > > CCBUG: 339193 > > > Diffs > - > > kio/kio/kfileitem.cpp f431d3608cfe646fb882365921e694af8ff8838f > > Diff: https://git.reviewboard.kde.org/r/122652/diff/ > > > Testing > --- > > dolphin remote: > -> no lock icon on smb:, mtp:, ... links > > > Thanks, > > Stefan Brüns > >
Re: Review Request 122652: Use correct default value when UDS_ACCESS/UDS_FILE_TYPE is not set
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122652/#review77642 --- Ship it! Ship It! - David Faure On March 17, 2015, 4:17 p.m., Stefan Brüns wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122652/ > --- > > (Updated March 17, 2015, 4:17 p.m.) > > > Review request for KDE Frameworks, kdelibs and David Faure. > > > Bugs: 339193 > http://bugs.kde.org/show_bug.cgi?id=339193 > > > Repository: kdelibs > > > Description > --- > > The default value for UDSEntry::numberValue(...) is 0, whereas KFileItem > uses the special value KFileItem::Unknown == (mode_t) -1. > > CCBUG: 339193 > > > Diffs > - > > kio/kio/kfileitem.cpp f431d3608cfe646fb882365921e694af8ff8838f > > Diff: https://git.reviewboard.kde.org/r/122652/diff/ > > > Testing > --- > > dolphin remote: > -> no lock icon on smb:, mtp:, ... links > > > Thanks, > > Stefan Brüns > >
Re: Syncing ECM release number with KF5
On Saturday 28 March 2015 05:49:01 Michael Palimaka wrote: > On 28/03/15 03:48, Alex Merry wrote: > > On Wednesday 25 March 2015 22:35:24 Stephen Kelly wrote: > >> Hello, > >> > >> ECM release numbers are in sync with KF5 release numbers, except for the > >> major component. > >> > >> This means that if you want to build the 5.x.y release you have to > >> download > >> the 1.x.y release of ECM. That doubles the complexity of your script > >> which > >> downloads the tarball to build it. > >> > >> That is bad and it is not necessary. > >> > >> Let's sync the major number for the next release. > >> > >> At some point the reason to make them out of sync was to be able to make > >> ECM releases more frequently. That is very rare because KF5 releases are > >> happening every month. If ECM needs to make an out of band release, it > >> can use the 4th version number component. > > > > I have no particular objection, although I think "doubling the complexity" > > of scripts is overstating things a little. > > > > Alex > > Is ECM actually part of KF5, or just happens to be released alongside > it? (I thought the latter, hence the different version). The initial idea was "it' happens to be released alongside, when necessary". I.e. as a "marketing" message: you can use ECM without using KF5. But, well, this modularity exists for the rest of KF5 too (you can use KArchive without using KIO), so this would still be clear if the version number was aligned. I also saw it as a way to not include it if it didn't change, but in practice there's always at least one change every month, usually more ;) > FWIW the different version doesn't bother me at all as a downstream. And it doesn't bother me as the release dude - I have one if() in the version file ;) (so, I also disagree with "doubles the complexity"). But since both Stephen Kelly and Alex Merry (maintainers of ECM) are in favour of switching, I'll make the switch. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 121080: Replace KDE_DUMMY_QHASH_FUNCTION.
> On Nov. 10, 2014, 9:41 p.m., David Faure wrote: > > lib/konq/src/konq_historyentry.h, line 57 > > <https://git.reviewboard.kde.org/r/121080/diff/1/?file=327432#file327432line57> > > > > const ref, no? > > Andrius da Costa Ribas wrote: > before I try to fix the pending issues: what are we going to decide? > > - Should we create KDE_DUMMY_QHASH_FUNCTION macro again? (which header) > - Should it apply to MSVC-only or should it be ifdef-free? Clearly this is not KDE specific. Any QList requires this on MSVC, right? Then I would strongly suggest a solution within Qt itself, *if* a central solution such as a macro is indeed needed. But thinking about it, a one-line dummy impl that returns 0 doesn't really seem worth a macro to me. I.e. why not do like qitemselectionmodel.h does, everywhere where this is needed? But I am still surprised that Qt only needs this in qitemselectionmodel.h Take this for instance: src/corelib/io/qstorageinfo.h:static QList mountedVolumes(); Why doesn't this require a qHash(QStorageInfo)? If I explicitly call toSet() on such a list, I do get a compile error (on Linux) due to the missing qHash implementation. http://www.davidfaure.fr/2015/storageview.diff So MSVC doesn't *always* instanciate the toSet() method, but only in some cases? Or are we looking at an old MSVC issue which doesn't exist anymore with more recent MSVC versions? i.e. did you try removing this block (in konq_historyentry.h) altogether to check it's still needed? - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121080/#review70214 --- On Nov. 8, 2014, 10:26 p.m., Andrius da Costa Ribas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121080/ > --- > > (Updated Nov. 8, 2014, 10:26 p.m.) > > > Review request for KDE Base Apps, KDE Frameworks and kdewin. > > > Repository: kde-baseapps > > > Description > --- > > Since we're not using kdemacros.h here anymore, KDE_DUMMY_QHASH_FUNCTION is > not available. Implement it instead. > > > Diffs > - > > lib/konq/src/konq_historyentry.h de34d6b > > Diff: https://git.reviewboard.kde.org/r/121080/diff/ > > > Testing > --- > > It builds (MSVC2013 - 64bit) after this patch (along other patches I'm > sending to review today). Kdebase-apps is still not very functional, though > (missing icons and weird UI). > > > Thanks, > > Andrius da Costa Ribas > >
Re: Review Request 117345: Fix crash in KIO due to exposing inconsistent views of internal data.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117345/#review79240 --- Should be committed to kdelibs4 indeed. Preferrably with the unittest improvements that you wrote for KF5. Sorry for the delay before I reviewed it. - David Faure On Dec. 19, 2014, 6:50 p.m., Simeon Bird wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/117345/ > --- > > (Updated Dec. 19, 2014, 6:50 p.m.) > > > Review request for kdelibs. > > > Repository: kdelibs > > > Description > --- > > Fix crash in KIO due to exposing inconsistent views of internal data. > > This can be triggered by renaming a directory while one of the files in it is > open in gwenview. > > It occurs because when KDirListerCache::emitRedirections is called, > itemsInUse contains the old url. However, KDirLister::Private::redirect > changes lstDirs to the new url. Thus at this point lstDirs contains an > item not in itemsInUse, which causes an assertion if forgetDirs is > called. > > In gwenview, the redirection signal is connected to openURL. This calls > forgetDirs, and causes the assertion. > > The solution: update itemsInUse *before* emitting redirections. > > This fixes the crash, but gwenview opens the first file in > the new directory instead of the file open before renaming. > This is probably an unrelated gwenview bug. > > I wrote this for kdelibs 4, but the code seems unchanged in kdelibs 5. > > > Diffs > - > > kio/kio/kdirlister.cpp aad6893f47eba81c3f78ed1ca7327adf6fb587bb > > Diff: https://git.reviewboard.kde.org/r/117345/diff/ > > > Testing > --- > > It fixes the crash! It might break something else. > > > Thanks, > > Simeon Bird > >
Re: Review Request 121080: Replace KDE_DUMMY_QHASH_FUNCTION.
> On Nov. 10, 2014, 9:41 p.m., David Faure wrote: > > lib/konq/src/konq_historyentry.h, line 57 > > <https://git.reviewboard.kde.org/r/121080/diff/1/?file=327432#file327432line57> > > > > const ref, no? > > Andrius da Costa Ribas wrote: > before I try to fix the pending issues: what are we going to decide? > > - Should we create KDE_DUMMY_QHASH_FUNCTION macro again? (which header) > - Should it apply to MSVC-only or should it be ifdef-free? > > David Faure wrote: > Clearly this is not KDE specific. Any QList requires this on > MSVC, right? Then I would strongly suggest a solution within Qt itself, *if* > a central solution such as a macro is indeed needed. But thinking about it, a > one-line dummy impl that returns 0 doesn't really seem worth a macro to me. > I.e. why not do like qitemselectionmodel.h does, everywhere where this is > needed? > > But I am still surprised that Qt only needs this in qitemselectionmodel.h > Take this for instance: > src/corelib/io/qstorageinfo.h:static QList > mountedVolumes(); > Why doesn't this require a qHash(QStorageInfo)? > If I explicitly call toSet() on such a list, I do get a compile error (on > Linux) due to the missing qHash implementation. > http://www.davidfaure.fr/2015/storageview.diff > So MSVC doesn't *always* instanciate the toSet() method, but only in some > cases? > Or are we looking at an old MSVC issue which doesn't exist anymore with > more recent MSVC versions? i.e. did you try removing this block (in > konq_historyentry.h) altogether to check it's still needed? > > Andrius da Costa Ribas wrote: > There is some historic reference here: > http://marc.info/?l=kde-core-devel&m=113069150408826&w=2 > > MSVC 2013 still has the same beahviour. I know it's because MSVC instanciates all methods. This is why I don't understand why it works for e.g. QStorageInfo. Sounds to me like further research is needed, I don't like fixes where we don't really understand what's going on. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121080/#review70214 --- On Nov. 8, 2014, 10:26 p.m., Andrius da Costa Ribas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121080/ > --- > > (Updated Nov. 8, 2014, 10:26 p.m.) > > > Review request for KDE Base Apps, KDE Frameworks and kdewin. > > > Repository: kde-baseapps > > > Description > --- > > Since we're not using kdemacros.h here anymore, KDE_DUMMY_QHASH_FUNCTION is > not available. Implement it instead. > > > Diffs > - > > lib/konq/src/konq_historyentry.h de34d6b > > Diff: https://git.reviewboard.kde.org/r/121080/diff/ > > > Testing > --- > > It builds (MSVC2013 - 64bit) after this patch (along other patches I'm > sending to review today). Kdebase-apps is still not very functional, though > (missing icons and weird UI). > > > Thanks, > > Andrius da Costa Ribas > >
KDE Frameworks 5.10.0 released
09th May 2015. KDE today announces the release of KDE Frameworks 5.10.0. KDE Frameworks are 60 addon libraries to Qt which provide a wide variety of· commonly needed functionality in mature, peer reviewed and well tested· libraries with friendly licensing terms. For an introduction see the· Frameworks 5.0 release announcement. KConfig Generate QML-proof classes using the kconfigcompiler KCoreAddons New cmake macro kcoreaddons_add_plugin to create KPluginLoader-based plugins more easily. KDeclarative Fix crash in texture cache. and other fixes KGlobalAccel Add new method globalShortcut which retrieves the shortcut as defined in global settings. KIdleTime Prevent kidletime from crashing on platform wayland KIO Added KPropertiesDialog::KPropertiesDialog(urls) and KPropertiesDialog::showDialog(urls). Asynchronous QIODevice-based data fetch for KIO::storedPut and KIO::AccessManager::put. Fix conditions with QFile::rename return value (bug 343329) Fixed KIO::suggestName to suggest better names (bug 341773) kioexec: Fixed path for writeable location for kurl (bug 343329) Store bookmarks only in user-places.xbel (bug 345174) Duplicate RecentDocuments entry if two different files have the same name Better error message if a single file is too large for the trash (bug 332692) Fix KDirLister crash upon redirection when the slot calls openURL KNewStuff New set of classes, called KMoreTools and related. KMoreTools helps to add hints about external tools which are potentially not yet installed. Furthermore, it makes long menus shorter by providing a main and more section which is also user-configurable. KNotifications Fix KNotifications when used with Ubuntu's NotifyOSD (bug 345973) Don't trigger notification updates when setting the same properties (bug 345973) Introduce LoopSound flag allowing notifications to play sound in a loop if they need it (bug 346148) Don't crash if notification doesn't have a widget KPackage Add a KPackage::findPackages function similar to KPluginLoader::findPlugins KPeople Use KPluginFactory for instantiating the plugins, instead of KService (kept for compatibility). KService Fix wrong splitting of entry path (bug 344614) KWallet Migration agent now also check old wallet is empty before starting (bug 346498) KWidgetsAddons KDateTimeEdit: Fix so user input actually gets registered. Fix double margins. KFontRequester: fix selecting monospaced fonts only KWindowSystem Don't depend on QX11Info in KXUtils::createPixmapFromHandle (bug 346496) new method NETWinInfo::xcbConnection() -> xcb_connection_t* KXmlGui Fix shortcuts when secondary shortcut set (bug 345411) Update list of bugzilla products/components for bug reporting (bug 346559) Global shortcuts: allow configuring also the alternate shortcut NetworkManagerQt The installed headers are now organized like all other frameworks. Plasma framework PlasmaComponents.Menu now supports sections Use KPluginLoader instead of ksycoca for loading C++ dataengines Consider visualParent rotation in popupPosition (bug 345787) Sonnet Don't try to highlight if there is no spell checker found. This would lead to an infinite loop with rehighlighRequest timer firing constanty. Frameworkintegration Fix native file dialogs from widgets QFileDialog: * File dialogs opened with exec() and without parent were opened, but any user-interaction was blocked in a way that no file could be selected nor the dialog closed. * File dialogs opened with open() or show() with parent were not opened at all. http://kde.org/announcements/kde-frameworks-5.10.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: building kio on Mac
That wasn't very constructive/positive... On Monday 08 June 2015 15:22:20 Ben Cooksley wrote: > The Qt developers > didn't want to provide any infrastructure at all to make developer > environments (including our CI system) easier. The *any* here is too broad. One approach was rejected, there are tons of others. E.g. just naming the variables QT_ instead of XDG_ might have been less controversial. But since everyone was saying, at the same time, that end users don't want env vars, I can understand that the Qt developers thought this issue needs more thinking, to solve all uses cases, not just "KDE CI" (which was a too restrictive line of argumentation compared to what it really was, "developer setup", as you say). > The maintainer(s) of > the QStandardPaths class never reviewed our patch That would be me, and since I don't know how things should work on OSX, I am not in a good position to decide. On top of that I come from the KDE world, so I can't really force a KDE patch into Qt if it's a bit controversial. > , and the module > maintainer for QtCore wanted the opinion of a Digia employee who was > extremely unresponsive. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
KDE Frameworks 5.11.0 released
12th June 2015. KDE today announces the release of KDE Frameworks 5.11.0. KDE Frameworks are 60 addon libraries to Qt which provide a wide variety of· commonly needed functionality in mature, peer reviewed and well tested· libraries with friendly licensing terms. For an introduction see the· Frameworks 5.0 release announcement. Extra CMake Modules New arguments for ecm_add_tests(). (bug 345797) Framework Integration Use the correct initialDirectory for the KDirSelectDialog Make sure the scheme is specified when overriding the start url value Only accept existing directories in FileMode::Directory mode KActivities (no changelog provided) KAuth Make KAUTH_HELPER_INSTALL_ABSOLUTE_DIR available to all KAuth users KCodecs KEmailAddress: Add overload for extractEmailAddress and firstEmailAddress which returns an error message. KCompletion Fix unwanted selection when editing the filename in the file dialog (bug 344525) KConfig Prevent crash if QWindow::screen() is null Add KConfigGui::setSessionConfig() (bug 346768) KCoreAddons New KPluginLoader::findPluginById() convenience API KDeclarative support creation of ConfigModule from KPluginMetdata fix pressAndhold events KDELibs 4 Support Use QTemporaryFile instead of hardcoding a temporary file. KDocTools Update translations Update customization/ru Fix entities with wrong links KEmoticons Cache the theme in the integration plugin KGlobalAccel [runtime] Move platform specific code into plugins KIconThemes Optimize KIconEngine::availableSizes() KIO Do not try to complete users and assert when prepend is non-empty. (bug 346920) Use KPluginLoader::factory() when loading KIO::DndPopupMenuPlugin Fix deadlock when using network proxies (bug 346214) Fixed KIO::suggestName to preserve file extensions Kick off kbuildsycoca4 when updating sycoca5. KFileWidget: Don't accept files in directory only mode KIO::AccessManager: Make it possible to treat sequential QIODevice asynchronously KNewStuff Add new method fillMenuFromGroupingNames KMoreTools: add many new groupings KMoreToolsMenuFactory: handling for "git-clients-and-actions" createMenuFromGroupingNames: make url parameter optional KNotification Fix crash in NotifyByExecute when no widget has been set (bug 348510) Improve handling of notifications being closed (bug 342752) Replace QDesktopWidget usage with QScreen Ensure KNotification can be used from a non-GUI thread Package Framework Guard the structure qpointer access (bug 347231) KPeople Use QTemporaryFile instead of hardcoding /tmp. KPty Use tcgetattr & tcsetattr if available Kross Fix loading of Kross modules "forms" and "kdetranslation" KService When running as root preserve file ownership on existing cache files (bug 342438) Guard against being unable to open stream (bug 342438) Fix check for invalid permissions writing file (bug 342438) Fix querying ksycoca for x-scheme-handler/* pseudo-mimetypes. (bug 347353) KTextEditor Allow like in KDE 4.x times 3rdparty apps/plugins to install own highlighting XML files into katepart5/syntax Add KTextEditor::Document::searchText() Bring back use of KEncodingFileDialog (bug 343255) KTextWidgets Add a method to clear decorator Allow to use custom sonnet decorator Implement "find previous" in KTextEdit. Re-add support for speech-to-text KWidgetsAddons KAssistantDialog: Re-add the Help button that was present in KDELibs4 version KXMLGUI Add session management for KMainWindow (bug 346768) NetworkManagerQt Drop WiMAX support for NM 1.2.0+ Plasma Framework Calendar components can now display week numbers (bug 338195) Use QtRendering for fonts in password fields Fix AssociatedApplicationManager lookup when a mimetype has (bug 340326) Fix panel background coloring (bug 347143) Get rid of "Could not load applet" message Capability to load QML kcms in plasmoid config windows Don't use the DataEngineStructure for Applets Port libplasma away from sycoca as much as possible [plasmacomponents] Make SectionScroller follow the ListView.section.criteria Scroll bars no longer automatically hide when a touch screen is present (bug 347254) Sonnet Use one central cache for the SpellerPlugins. Reduce temporary allocations. Optimize: Do not wipe dict cache when copying speller objects. Optimise away save() calls by calling it once at the end if needed. http://kde.org/announcements/kde-frameworks-5.11.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KSyCoca, Thread safety, and Cache invalidation
On Saturday 13 June 2015 02:04:03 Vishesh Handa wrote: > 3. The gui thread on receiving the dbus signal updates its db as well > as the database of all other threads. This is slightly complex and > would require locking code similar to (2) since the other threads > could be in the process of using KSycoca. 3 bis: I assume the threads without an event loop have some way to get tasks, right? So when the gui thread gets notified about ksycoca changes, it should post a task to these threads-with-no-eventloop which says "sycoca has changed". It's ok if they keep using an old instance meanwhile (the mmap'ed file uses refcounting in the kernel). When the thread finally gets the task it can call KSycoca::notifyDatabaseChanged (it's a private slot, use QMetaObject::invokeMethod for now, if it works we can make it public). -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 123965: Encode KBookmark URL to fix compatibility with KDE4 applications
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123965/#review81471
---
Ship it!
I have no problems with using FullyEncoded in the file, but this is still
surprising, Qt5's toString should be a valid URL, parsable by KDE4's KUrl, I
would have thought. Oh well.
- David Faure
On May 31, 2015, 1:28 p.m., Andrey Bondrov wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123965/
> ---
>
> (Updated May 31, 2015, 1:28 p.m.)
>
>
> Review request for Dolphin, kdelibs, David Faure, and Hrvoje Senjan.
>
>
> Repository: kbookmarks
>
>
> Description
> ---
>
> We need to encode KBookmark URL to fix compatibility with KDE4 applications.
>
> For example, KDE4 stores "~/" ("Downloads" in Russian) as
>
> bookmark
> href="file:///home/vuohi/%D0%97%D0%B0%D0%B3%D1%80%D1%83%D0%B7%D0%BA%D0%B8/"
>
> while KF5 stores "~/" as
>
> bookmark href="file:///home/vuohi//"
>
> When the bookmark is created with KF5, KDE4 applications cannot open it.
>
> P.S. Looks like this editor cannot display non-latin characters, it replaced
> actual string with questions.
>
>
> Diffs
> -
>
> src/kbookmark.cpp 98ddc99
>
> Diff: https://git.reviewboard.kde.org/r/123965/diff/
>
>
> Testing
> ---
>
> Only quick testing is done. I deleted user-places.xbel, added bookmark with
> kwrite (KF5 version), checked if it's properly stored in new user-places.xbel
> and if Dolphin (KDE4 build) can open it from Places panel. Seems to work
> properly.
>
>
> Thanks,
>
> Andrey Bondrov
>
>
Re: KSyCoca, Thread safety, and Cache invalidation
On Friday 19 June 2015 22:00:09 Vishesh Handa wrote: > On Sat, Jun 13, 2015 at 9:26 PM, David Faure wrote: > > 3 bis: > > I assume the threads without an event loop have some way to get tasks, > > right? So when the gui thread gets notified about ksycoca changes, it > > should post a task to these threads-with-no-eventloop which says "sycoca > > has changed". It's ok if they keep using an old instance meanwhile (the > > mmap'ed file uses refcounting in the kernel). > > When the thread finally gets the task it can call > > KSycoca::notifyDatabaseChanged (it's a private slot, use > > QMetaObject::invokeMethod for now, if it works we can make it public). > > I've tried a hackish way to communicate with the other threads. See > attached patch. > > This doesn't quite work since KSysCocaPrivate::checkDatabase isn't > always called before finding an entry. Resetting the db in findEntry > leads of crashes cause the state of the stream isn't as expected by > the factory. > > Any tips? I thought about it again, and there's a much simpler solution. We get rid of dbus notification altogether, and we simply close+reopen the database if its mtime changed. This is how things work for shared-mime-info (QMimeDatabase) and it's much simpler. This requires adding a check for whether ksycoca is uptodate before any use of ksycoca data, maybe this can be done in KServiceFactory::self() and similar (assuming nobody ever caches that value, but none of the code in kservice does AFAICS). By having the mtime in ksycoca, it'll be per-thread, and no threading-specific code will be required at all. This would be more in line with the long term plans for all this, too (which is exactly that: something that works more like shared-mime-info: a per- directory cache updated at install time and on demand if the mtime or the directory changed, and the way to notice that the cache changed is by noticing its own mtime changing). -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
KDE Frameworks 5.12.0 released
10th July 2015. KDE today announces the release of KDE Frameworks 5.12.0. KDE Frameworks are 60 addon libraries to Qt which provide a wide variety of· commonly needed functionality in mature, peer reviewed and well tested· libraries with friendly licensing terms. For an introduction see the· Frameworks 5.0 release announcement. Extra CMake Modules Improve error reporting of query_qmake macro BluezQt Remove all devices from adapter before removing the adapter (bug 349363) Update links in README.md KActivities Adding the option not to track the user when in specific activities (similar to the 'private browsing' mode in a web browser) KArchive Preserve executable permissions from files in copyTo() Clarify ~KArchive by removing dead code. KAuth Make it possible to use kauth-policy-gen from different sources KBookmarks Don't add a bookmark with url is empty and text is empty Encode KBookmark URL to fix compatibility with KDE4 applications KCodecs Remove x-euc-tw prober KConfig Install kconfig_compiler into libexec New code generation option TranslationDomain=, for use with TranslationSystem=kde; normally needed in libraries. Make it possible to use kconfig_compiler from different sources KCoreAddons KDirWatch: Only establish a connection to FAM if requested Allow filtering plugins and applications by formfactor Make it possible to use desktoptojson from different sources KDBusAddons Clarify exit value for Unique instances KDeclarative Add QQC clone of KColorButton Assign a QmlObject for each kdeclarative instance when possible Make Qt.quit() from QML code work Merge branch 'mart/singleQmlEngineExperiment' Implement sizeHint based on implicitWidth/height Subclass of QmlObject with static engine KDELibs 4 Support Fix KMimeType::Ptr::isNull implementation. Reenable support for KDateTime streaming to kDebug/qDebug, for more SC Load correct translation catalog for kdebugdialog Don't skip documenting deprecated methods, so that people can read the porting hints KDESU Fix CMakeLists.txt to pass KDESU_USE_SUDO_DEFAULT to the compilation so it is used by suprocess.cpp KDocTools Update K5 docbook templates KGlobalAccel private runtime API gets installed to allow KWin to provide plugin for Wayland. Fallback for componentFriendlyForAction name resolving KIconThemes Don't try to paint the icon if the size is invalid KItemModels New proxy model: KRearrangeColumnsProxyModel. It supports reordering and hiding columns from the source model. KNotification Fix pixmap types in org.kde.StatusNotifierItem.xml [ksni] Add method to retrieve action by its name (bug 349513) KPeople Implement PersonsModel filtering facilities KPlotting KPlotWidget: add setAutoDeletePlotObjects, fix memory leak in replacePlotObject Fix missing tickmarks when x0 > 0. KPlotWidget: no need to setMinimumSize or resize. KTextEditor debianchangelog.xml: add Debian/Stretch, Debian/Buster, Ubuntu-Wily Fix for UTF-16 surrogate pair backspace/delete behavior. Let QScrollBar handle the WheelEvents (bug 340936) Apply patch from KWrite devel top update pure basic HL, "Alexander Clay" KTextWidgets Fix enable/disable ok button KWallet Framework Imported and improved the kwallet-query command-line tool. Support to overwrite maps entries. KXMLGUI Don't show "KDE Frameworks version" in the About KDE dialog Plasma Framework Make the dark theme completely dark, also the complementary group Cache naturalsize separately by scalefactor ContainmentView: Do not crash on an invalid corona metadata AppletQuickItem: Do not access KPluginInfo if not valid Fix occasional empty applet config pages (bug 349250) Improve hidpi support in the Calendar grid component Verify KService has valid plugin info before using it [calendar] Ensure the grid is repainted on theme changes [calendar] Always start counting weeks from Monday (bug 349044) [calendar] Repaint the grid when show week numbers setting changes An opaque theme is now used when only the blur effect is available (bug 348154) Whitelist applets/versions for separate engine Introduce a new class ContainmentView Sonnet Allow to use highlight spellchecking in a QPainTextEdit http://kde.org/announcements/kde-frameworks-5.12.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KSyCoca, Thread safety, and Cache invalidation
On Friday 26 June 2015 18:03:00 Frank Reininghaus wrote: > checking the mtime frequently can have a bad effect on the performance > though if it's done incorrectly. There is a bug report about high CPU > usage in Dolphin if "sort by type" is used: > > https://bugs.kde.org/show_bug.cgi?id=346974 > > According to the backtrace, the process is busy inside > QMimeDataBase::mimeTypeForName(QString) doing time-related things and > accessing /etc/localtime all the time, probably because of the mtime > check that you mentioned. I don't know that Qt version was used > though, so I'm not sure if that still applies to the most recent > versions. > > (I cannot quite reproduce the problem though because there are other > parts of Qt's mime type handling which keep my CPU even more busy than > the time check. I should check if this still happens in recent git > snapshots of Qt and try to investigate this further...) The mimetype code in Qt hasn't really changed, you won't get a difference by upgrading Qt. Interesting find, though. Maybe we could check the mtime only if more than N seconds have passed since the last check, but I wonder if even checking that (e.g. with QElapsedTimer) has a cost in terms of syscalls... -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KSyCoca, Thread safety, and Cache invalidation
On Friday 26 June 2015 18:03:00 Frank Reininghaus wrote:
> https://bugs.kde.org/show_bug.cgi?id=346974
>
> According to the backtrace, the process is busy inside
> QMimeDataBase::mimeTypeForName(QString) doing time-related things and
> accessing /etc/localtime all the time, probably because of the mtime
> check that you mentioned. I don't know that Qt version was used
> though, so I'm not sure if that still applies to the most recent
> versions.
I forgot my own code, it seems.
Looking at it again: it does already only check the mtime on disk every 5
seconds.
int qmime_secondsBetweenChecks = 5;
bool QMimeProviderBase::shouldCheck()
{
const QDateTime now = QDateTime::currentDateTime();
if (m_lastCheck.isValid() && m_lastCheck.secsTo(now) <
qmime_secondsBetweenChecks)
return false;
m_lastCheck = now;
return true;
}
But it's exactly that call to QDateTime::currentDateTime() which ends up
calling tzset
(which seems to access to /etc/localtime on disk every time), according to
https://bugsfiles.kde.org/attachment.cgi?id=92719
Maybe this code should use QElapsedTimer instead of
QDateTime::currentDateTime()?
Or maybe QDateTime::currentDateTime() could avoid calling the awful tzset()?
Thiago, any input?
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
Re: KSyCoca, Thread safety, and Cache invalidation
On Monday 20 July 2015 16:05:06 David Faure wrote:
> On Friday 26 June 2015 18:03:00 Frank Reininghaus wrote:
> > https://bugs.kde.org/show_bug.cgi?id=346974
> >
> > According to the backtrace, the process is busy inside
> > QMimeDataBase::mimeTypeForName(QString) doing time-related things and
> > accessing /etc/localtime all the time, probably because of the mtime
> > check that you mentioned. I don't know that Qt version was used
> > though, so I'm not sure if that still applies to the most recent
> > versions.
>
> I forgot my own code, it seems.
> Looking at it again: it does already only check the mtime on disk every 5
> seconds.
>
> int qmime_secondsBetweenChecks = 5;
> bool QMimeProviderBase::shouldCheck()
> {
> const QDateTime now = QDateTime::currentDateTime();
> if (m_lastCheck.isValid() && m_lastCheck.secsTo(now) <
> qmime_secondsBetweenChecks) return false;
> m_lastCheck = now;
> return true;
> }
>
> But it's exactly that call to QDateTime::currentDateTime() which ends up
> calling tzset (which seems to access to /etc/localtime on disk every time),
> according to https://bugsfiles.kde.org/attachment.cgi?id=92719
>
> Maybe this code should use QElapsedTimer instead of
> QDateTime::currentDateTime()? Or maybe QDateTime::currentDateTime() could
> avoid calling the awful tzset()? Thiago, any input?
Sorry, I had missed the other replies to this thread.
(I recommend using k-f-d to discuss code in KF5, kde-core-devel has too much
other stuff I have a hard time following it).
I'll work on a Qt patch.
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
Re: Fwd: [Krusader-devel] Help needed for Krusader .desktop files
On Tuesday 21 July 2015 13:41:47 Yuri Chornoivan wrote: > However, there's a "red cell" in the Krusader row, and it says > application .desktop filenames not yet updated to 'org.kde.' schema > > http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s02.html Yes, exactly. > Has anyone dealt with something similar before or can solve that > application .desktop filenames not yet updated to 'org.kde.' schema > step? Well, it's clear, isn't it? Rename your desktop file to org.kde.krusader.desktop, it's as easy as that. It will help determining the desktop file for a running app based on the executable name, which is a use case that shows up more and more (on X11 and on wayland). -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KSyCoca, Thread safety, and Cache invalidation
On Tuesday 14 July 2015 19:28:58 Thomas Lübking wrote: > On Dienstag, 14. Juli 2015 19:00:14 CEST, Milian Wolff wrote: > > On Tuesday, July 14, 2015 11:49:25 AM David Faure wrote: > >> On Friday 26 June 2015 18:03:00 Frank Reininghaus wrote: ... > > > > It has. Querying the current time repeatedly is quite costly, > > and often shows > > up in code that extensively uses timers, or > > QDateTime::currentDateTime() etc. > > > > But it's probably still cheaper than querying on-disk meta data > > in the worst > > case, esp. on old rotary disks. > > Actually, checking currentTime() is already the problem here (causing the IO > for the timezone stuff), see > http://marc.info/?l=kde-core-devel&m=143533622526705&w=1 (the 1st paragraph > part of my comment somehow turned into a second-level quote) > > Comparing a monotic timer (QElapsedTime) is however - at least on linux - > close to cost free PLUS: one can have an entirely free "do not check again > during this event cycle" flag. I used QElapsedTimer in https://codereview.qt-project.org/122247 and the resulting performance improvements are amazing. kbuildsycoca5 is twice faster. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
[kde-dev-scripts] /: Add support for ninja to makeobj
Git commit cd599de58b44596a5e86a5fe560e2904ef5efe7f by David Faure.
Committed on 30/07/2015 at 12:54.
Pushed by dfaure into branch 'master'.
Add support for ninja to makeobj
Usage: alias make=makeobj, as usual.
This allows to do "make" and "make install" (etc.) in either the source
dir or the build dir, provided that $OBJ_REPLACEMENT is set.
CCMAIL: m...@vhanda.in, kde-core-devel@kde.org
M +28 -9makeobj
http://commits.kde.org/kde-dev-scripts/cd599de58b44596a5e86a5fe560e2904ef5efe7f
diff --git a/makeobj b/makeobj
index d4f82ad..e4d65d4 100755
--- a/makeobj
+++ b/makeobj
@@ -111,10 +111,14 @@ dir=.
cwd=$PWD
# No CMakeList and no Makefile (and no .pro file either)? Maybe we need to go
up then.
-while test ! -f CMakeLists.txt && test ! -f Makefile; do
+while test ! -f CMakeLists.txt && test ! -f Makefile ; do
if test "`ls -1 *.pro 2>/dev/null`" && test -n "`ls -1 ../*.pro
2>/dev/null`"; then
break;
fi
+if test -f build.ninja; then
+file=build.ninja
+break;
+fi
dir="$dir/`basename \"$PWD\"`"
cd ..
if test X"$PWD" = X"/"; then
@@ -173,12 +177,23 @@ if test ! -f "$file"; then
sed -e s,%ARCH%,\"$_arch\",g | \
sed -e s,%OS%,\"$_os\",g`"
pwd="`echo $PWD | sed -e \"$OBJ_REPLACEMENT\"`"
+ if test ! -e $pwd; then
+ echo "no objdir found. Tried $pwd"
+ exit 1
+ fi
if test ! -f "$pwd/$file"; then
- # No objdir found. But if "make" will work in srcdir, then go ahead;
might be a non-kde project.
+ # ninja requires building from the toplevel
+ cd -- "$pwd"
+ findup build.ninja
+ if test -n "$_hit"; then
+ pwd=`dirname $_hit`
+ file=build.ninja
+ fi
+ # No objdir with a Makefile found. But if "make" will work in srcdir,
then go ahead; might be a non-kde project.
test -f "$pwd/GNUmakefile" && file=GNUmakefile
test -f "$pwd/makefile" && file=makefile
if ! test -f "$pwd/$file"; then
- echo "no objdir found. Tried $pwd"
+ echo "no Makefile or build.ninja found in $pwd"
exit 1
fi
fi
@@ -199,13 +214,17 @@ if test -z "$MAKE"; then
fi
if test $using_new_unsermake -eq 1; then
MAKE="`command -v unsermake`"
- if test ! -x "$MAKE"; then
- echo 'Makefile was created with unsermake, but there'
- echo 'is no unsermake in $PATH'
- exit 1
- fi
-else
+if test ! -x "$MAKE"; then
+echo 'Makefile was created with unsermake, but there'
+echo 'is no unsermake in $PATH'
+exit 1
+fi
+elif test -f "Makefile"; then
MAKE="$GMAKE"
+elif test -f "build.ninja"; then
+MAKE="ninja"
+else
+echo "No Makefile or build.ninja found in $PWD!"
fi
fi
LANG=en_US.UTF-8 $MAKE "${args[@]}"
Re: KSyCoca, Thread safety, and Cache invalidation
On Thursday 30 July 2015 21:23:00 Mark Gaiser wrote: > You can ignore this message if QMimeType::inherits internally calls the > method you optimized thus this function would be greatly improved already. It does. Why not apply existing known fixes before profiling for more issues? :) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
[kdbusaddons] src: KDBusService: document how to raise the active window, in Activate()
Git commit d9a6ce124ee6ece84e6af4c741f2ad0ed2599928 by David Faure. Committed on 03/08/2015 at 23:23. Pushed by dfaure into branch 'master'. KDBusService: document how to raise the active window, in Activate() CCMAIL: kde-core-devel@kde.org M +3-0src/kdbusservice.h http://commits.kde.org/kdbusaddons/d9a6ce124ee6ece84e6af4c741f2ad0ed2599928 diff --git a/src/kdbusservice.h b/src/kdbusservice.h index add327e..fedcf11 100644 --- a/src/kdbusservice.h +++ b/src/kdbusservice.h @@ -203,6 +203,9 @@ Q_SIGNALS: * @code *commandLineParser->parse(arguments); // same QCommandLineParser instance as the one used in main() *handleCmdLine(workingDirectory); // shared method with main(), which uses commandLineParser to handle options and positional arguments + *// and for GUI applications, also terminate startup notification and activate the mainwindow: + *KStartupInfo::setNewStartupId(mainWindow, KStartupInfo::startupId()); + *KWindowSystem::forceActiveWindow(mainWindow->winId()); * @endcode * * @see setExitValue()
Re: Review Request 124657: Show URL again in bookmarks view and URL texbox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124657/#review83584 --- Ship it! Ship It! - David Faure On Aug. 8, 2015, 2:24 p.m., Luigi Toscano wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124657/ > --- > > (Updated Aug. 8, 2015, 2:24 p.m.) > > > Review request for KDE Base Apps and David Faure. > > > Repository: kde-baseapps > > > Description > --- > > Fix two KUrl porting TODO items. Thanks to the porting notes! > The URL of the bookmark was not shown because the value was not > passed to the treeview/table model and to the textbox. > > > Diffs > - > > keditbookmarks/bookmarkinfowidget.cpp 98ce303 > keditbookmarks/kbookmarkmodel/model.cpp e949469 > > Diff: https://git.reviewboard.kde.org/r/124657/diff/ > > > Testing > --- > > Compiled, executed the bookmark editor. Added and changed few bookmarks, the > URL is correctly updated. > > > Thanks, > > Luigi Toscano > >
KDE Frameworks 5.13.0 released
issues vimode: don't crash if the command gets executed in the end of a document. (bug 350299) Support QML multi-line strings. fix syntax of oors.xml add CartoCSS hl by Lukas Sommer (bug 340756) fix floating point HL, use the inbuilt Float like in C (bug 348843) split directions did got reversed (bug 348845) Bug 348317 - [PATCH] Katepart syntax highlighting should recognize \u0123 style escapes for JavaScript (bug 348317) add *.cljs (bug 349844) Update the GLSL highlighting file. fixed default colors to be more distinguishable KTextWidgets Delete old highlighter KWallet Framework Fix Windows build Print a warning with error code when opening the wallet by PAM fails Return the backend error code rather than -1 when opening a wallet failed Make the backend's "unknown cipher" a negative return code Watch for PAM_KWALLET5_LOGIN for KWallet5 Fix crash when MigrationAgent::isEmptyOldWallet() check fails KWallet can now be unlocked by PAM using kwallet-pam module KWidgetsAddons New API taking QIcon parameters to set the icons in the tab bar KCharSelect: Fix unicode category and use boundingRect for width calculation KCharSelect: fix cell width to fit contents KMultiTabBar margins now are ok on HiDPI screens KRuler: deprecate unimplemented KRuler::setFrameStyle(), clean up comments KEditListWidget: remove margin, so it aligns better with other widgets KWindowSystem Harden NETWM data reading (bug 350173) guard for older Qt versions like in kio-http Private headers for platform plugins are installed. Platform specific parts loaded as plugins. KXMLGUI Fix method behavior KShortcutsEditorPrivate::importConfiguration Plasma Framework Using a pinch gesture one can now switch between the different zoom levels of the calenda comment about code duplication in icondialog Slider groove color was hardcoded, modified to use color scheme Use QBENCHMARK instead of a hard requirement on the machine's performance Calendar navigation has been significantly improved, providing a year and decade overview PlasmaCore.Dialog now has an 'opacity' property Make some space for the radio button Don't show the circular background if there's a menu Add X-Plasma-NotificationAreaCategory definition Set notifications and osd to show on all desktops Print useful warning when we can not get valid KPluginInfo Fix potential endless recursion in PlatformStatus::findLookAndFeelPackage() Rename software-updates.svgz to software.svgz Sonnet Add in CMake bits to enable building of Voikko plugin. Implement Sonnet::Client factory for Voikko spell chekers. Implement Voikko based spell checker (Sonnet::SpellerPlugin) http://kde.org/announcements/kde-frameworks-5.13.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Proposal to improving KDE Software Repository Organization
w we release things. Nowadays we basically have 4 products (frameworks, plasma, applications, extragear?), previously we had 2 (KDE SC 4, extragear). If this is what you had in mind with divisions, then I suggest to call it product for clarity. The reason why I think it's the same concept is that the one reason to have different tracks is exactly because of different release schedules (e.g. plasma could be tested against KF5/Stable (last release) and KF5/Devel (more recent code)) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: What to do with our default hidden visibility for symbols?
On Friday 14 August 2015 11:01:47 Jaroslaw Staniek wrote: > > This raises a warning [1] for CMake 3.3+ and ignores this. > Don't we want to set the old policy CMP0063 for now? > Or even prepare for removal of CMP0063 (see [2])? > I know, that would not be very soon. This discussion is on-going on kde-buildsys...@kde.org, which is the proper place for it. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Proposal to improving KDE Software Repository Organization
On Sunday 16 August 2015 13:51:29 Michael Pyne wrote: > There's no reason even with our current build metadata that we'd *have* to > have project hierarchies, as long as each underlying git repository name > remains unique. It might even make things easier since there would be no way > for a sub-path in our project hierarchy (such as kde/kdelibs/kactivities) to > mask a git repository name (kdelibs in this case). Ben and I discussed it today and we think there is usefulness in one level of subtree within the Applications product, to be able to keep the 'groups' like kdegraphics, kdemultimedia etc. which are useful in order to have a maintainer per 'group' (as reinforced by the release team recently). But yes, only one level, and AFAICS only useful in Applications. kactivities (to pick your example) would be "at the root of" Frameworks, no sub-path needed. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Proposal to improving KDE Software Repository Organization
On Sunday 16 August 2015 23:36:33 Luigi Toscano wrote: > David Faure ha scritto: > > On Sunday 16 August 2015 13:51:29 Michael Pyne wrote: > >> There's no reason even with our current build metadata that we'd *have* to > >> have project hierarchies, as long as each underlying git repository name > >> remains unique. It might even make things easier since there would be no > >> way > >> for a sub-path in our project hierarchy (such as kde/kdelibs/kactivities) > >> to > >> mask a git repository name (kdelibs in this case). > > > > Ben and I discussed it today and we think there is usefulness in one level > > of subtree within the > > Applications product, to be able to keep the 'groups' like kdegraphics, > > kdemultimedia etc. which > > are useful in order to have a maintainer per 'group' (as reinforced by the > > release team recently). > > > > But yes, only one level, and AFAICS only useful in Applications. > > kactivities (to pick your example) would be "at the root of" Frameworks, no > > sub-path needed. > > > Does it mean a giant big blob for extragear and playground? Translation-wise, > having the 'groups' is really useful to not get lost. > > Also, when phabricator support subproject, using groups would be useful again > to not have a big blob of projects (it was one of the few complains I recorded > for phabricator, the big list of projects). OK, so more products with repos organized in sub-paths, makes sense to me. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Best-practise currently for testing internal parts of libs? *_TEST_EXPORT macro?
On Monday 31 August 2015 14:41:00 Friedrich W. H. Kossebau wrote: > > The only place lxr.kde.org pointed out to use the *TEST_EXPORT approach was > grantlee, which simply creates a separate file with the define that then is > appended to the file generated with generate_export_header: > http://lxr.kde.org/source/grantlee/templates/lib/CMakeLists.txt Konqueror has a simpler approach, a file that includes the generated file and adds the tests_export macro on top. #include "konquerorprivate_export.h" /* Classes from the Konqueror application, which are exported only for unit tests */ #ifdef COMPILING_TESTS # ifndef KONQ_TESTS_EXPORT # define KONQ_TESTS_EXPORT KONQUERORPRIVATE_EXPORT # endif #else /* not compiling tests */ # define KONQ_TESTS_EXPORT #endif See kde/applications/kde-baseapps/konqueror/src/konqprivate_export.h branch frameworks. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 125010: konq-plugins: port kttsd plugin to QtSpeech
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125010/#review84941 --- Ship it! Ah, sorry. I skipped reviewing that one because I was thinking "well, I don't know anything about QtSpeech. But indeed the QtSpeec part of it is a trivial two-liner :-) konq-plugins/CMakeLists.txt (line 21) <https://git.reviewboard.kde.org/r/125010/#comment58754> using macro_log_feature would be better konq-plugins/ttsplugin/khtmltts.cpp (line 30) <https://git.reviewboard.kde.org/r/125010/#comment58753> unused konq-plugins/ttsplugin/khtmltts.cpp (line 51) <https://git.reviewboard.kde.org/r/125010/#comment58752> This line seems unused (in which case, remove the comment for it too). - David Faure On Sept. 1, 2015, 2:23 a.m., Jeremy Whiting wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125010/ > --- > > (Updated Sept. 1, 2015, 2:23 a.m.) > > > Review request for kdelibs and David Faure. > > > Repository: kde-baseapps > > > Description > --- > > Ported kttsd plugin to QtSpeech. > > > Diffs > - > > konq-plugins/CMakeLists.txt 1d74e91 > konq-plugins/kttsplugin/CMakeLists.txt 2faa7e6 > konq-plugins/kttsplugin/Messages.sh 3c21b1c > konq-plugins/kttsplugin/khtmlkttsd.cpp c361321 > konq-plugins/kttsplugin/khtmlkttsd.desktop cb0a4bd > konq-plugins/kttsplugin/khtmlkttsd.h 99bc848 > konq-plugins/kttsplugin/khtmlkttsd.rc 75a2c49 > konq-plugins/ttsplugin/CMakeLists.txt PRE-CREATION > konq-plugins/ttsplugin/Messages.sh PRE-CREATION > konq-plugins/ttsplugin/khtmltts.cpp PRE-CREATION > konq-plugins/ttsplugin/khtmltts.desktop PRE-CREATION > konq-plugins/ttsplugin/khtmltts.h PRE-CREATION > konq-plugins/ttsplugin/khtmltts.rc PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/125010/diff/ > > > Testing > --- > > Builds, and works (wow I haven't ran konqueror in ages...) > > > Thanks, > > Jeremy Whiting > >
Re: Review Request 125010: konq-plugins: port kttsd plugin to QtSpeech
> On Sept. 7, 2015, 7:37 a.m., David Faure wrote: > > konq-plugins/CMakeLists.txt, line 21 > > <https://git.reviewboard.kde.org/r/125010/diff/1/?file=399825#file399825line21> > > > > using macro_log_feature would be better > > Jeremy Whiting wrote: > Hmm, did macro_log_feature get replaced by something else? I can't seem > to find it actually used in any kf5 based framework or application here > somehow. Doh sorry, forgot to update my brain. I meant set_package_properties (see KIO for an example). - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125010/#review84941 --- On Sept. 8, 2015, 3:42 p.m., Jeremy Whiting wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125010/ > --- > > (Updated Sept. 8, 2015, 3:42 p.m.) > > > Review request for kdelibs and David Faure. > > > Repository: kde-baseapps > > > Description > --- > > Ported kttsd plugin to QtSpeech. > > > Diffs > - > > konq-plugins/CMakeLists.txt 1d74e91 > konq-plugins/kttsplugin/CMakeLists.txt 2faa7e6 > konq-plugins/kttsplugin/Messages.sh 3c21b1c > konq-plugins/kttsplugin/khtmlkttsd.cpp c361321 > konq-plugins/kttsplugin/khtmlkttsd.desktop cb0a4bd > konq-plugins/kttsplugin/khtmlkttsd.h 99bc848 > konq-plugins/kttsplugin/khtmlkttsd.rc 75a2c49 > konq-plugins/ttsplugin/CMakeLists.txt PRE-CREATION > konq-plugins/ttsplugin/Messages.sh PRE-CREATION > konq-plugins/ttsplugin/khtmltts.cpp PRE-CREATION > konq-plugins/ttsplugin/khtmltts.desktop PRE-CREATION > konq-plugins/ttsplugin/khtmltts.h PRE-CREATION > konq-plugins/ttsplugin/khtmltts.rc PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/125010/diff/ > > > Testing > --- > > Builds, and works (wow I haven't ran konqueror in ages...) > > > Thanks, > > Jeremy Whiting > >
KDE Frameworks 5.14.0 released
pletion pop-up appearance minimap: Attempt to improve the look and feel (bug 309553) nested comments in Haskell syntax highlighting Fix problem with wrong unindent for python (bug 351190) KWidgetsAddons KPasswordDialog: let the user change the password visibility (bug 224686) KXMLGUI Fix KSwitchLanguageDialog not showing most languages KXmlRpcClient Avoid QLatin1String wherever it allocates heap memory ModemManagerQt Fix metatype conflict with the latest nm-qt change NetworkManagerQt Added new properties from the latest NM snapshot/releases Plasma Framework reparent to flickable if possible fix package listing plasma: Fix applet actions might be nullptr (bug 351777) The onClicked signal of PlasmaComponents.ModelContextMenu now works properly PlasmaComponents ModelContextMenu can now create Menu sections Port platformstatus kded plugin to json metadata... Handle an invalid metadata in PluginLoader Let the RowLayout figure out the size of the label always show the edit menu when the cursor is visible Fix loop on ButtonStyle Don't change the flat-iness of a button on pressed on touchscreen and mobile scrollbars are transient adjust flick velocity&deceleration to dpi custom cursor delegate only if mobile touch friendly text cursor fix parenting and popping up policy declare __editMenu add missing cursot handles delegates rewrite the EditMenu implementation use the mobile menu only conditionally reparent the menu to root http://kde.org/announcements/kde-frameworks-5.14.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: SIC/BIC in Frameworks (NOT)
This is clearly a CI issue, not a SIC/BIC, as both our investigations prove. On Sunday 13 September 2015 23:43:19 Ben Cooksley wrote: > This then causes KNotification to fail to build (as it depends on > netwm.h) which then leads to the rest of the failures we see. This > itself is probably a bug? If KWindowSystem is built with X11 forced OFF (via CMAKE_DISABLE_FIND_PACKAGE_X11) and KNotifications is built with X11 enabled (CMAKE_DISABLE_FIND_PACKAGE_X11 not being set), you end up with a compilation error. I think that's quite obvious, and I wouldn't consider that a bug. We only support X11 on everywhere or off everywhere. Combinations of "some frameworks with X11 enabled and some with X11 purposefully disabled" don't make sense to me. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KSyCoca, Thread safety, and Cache invalidation
On Tuesday 14 July 2015 16:01:09 Thiago Macieira wrote:
>
> If you need a machine-comparable time with other systems or across reboots,
> use QDateTime::currentDateTimeUtc(). That avoids refreshing the timezone
> database to convert to local time.
>
> Only use QDateTime::currentDateTime() if you want to show something to the
> user. There's no other valid reason. (writing to a log counts as "showing to
> the user")
What if I need to compare a file's modification time with a given timestamp?
This code:
qCDebug(SYCOCA) << "checking file timestamps";
const QDateTime stamp = QDateTime::fromMSecsSinceEpoch(timestamp);
for (QStringList::ConstIterator it = dirs.begin(); it != dirs.end(); ++it) {
const QString dir = *it;
QFileInfo inf(dir);
if (inf.lastModified() > stamp) {
qCDebug(SYCOCA) << "timestamp changed:" << dir;
return false;
}
... doesn't appear to be threadsafe due to QDateTime's timezone conversion.
It's really a problem that QDateTime isn't threadsafe, it's really unexpected
for application programmers.
Helgrind says:
==24943== Possible data race during read of size 8 at 0x67A5AC0 by thread #1
==24943== Locks held: none
==24943==at 0x573B068: qt_timezone() (qdatetime.cpp:2153)
==24943==by 0x573BA87: localMSecsToEpochMSecs(long long,
QDateTimePrivate::DaylightStatus*, QDate*, QTime*, QString*)
(qdatetime.cpp:2480)
==24943==by 0x5741BAB: QDateTimePrivate::toMSecsSinceEpoch() const
(qdatetime.cpp:2685)
==24943==by 0x573CEFB: QDateTime::toMSecsSinceEpoch() const
(qdatetime.cpp:3346)
==24943==by 0x573E8F7: QDateTime::operator<(QDateTime const&) const
(qdatetime.cpp:3989)
==24943==by 0x4E9832C: QDateTime::operator>(QDateTime const&) const
(qdatetime.h:283)
==24943==by 0x4E96980: checkTimestamps(long long, QStringList const&)
(ksycoca.cpp:571)
==24943== This conflicts with a previous write of size 8 by thread #2
==24943== Locks held: none
==24943==at 0x64AC567: __tzfile_compute (tzfile.c:773)
==24943==by 0x64AB2E8: __tz_convert (tzset.c:635)
==24943==by 0x573B4ED: qt_localtime(long long, QDate*, QTime*,
QDateTimePrivate::DaylightStatus*) (qdatetime.cpp:2337)
==24943==by 0x573B95D: epochMSecsToLocalTime(long long, QDate*, QTime*,
QDateTimePrivate::DaylightStatus*) (qdatetime.cpp:2440)
==24943==by 0x573D1D8: QDateTime::setMSecsSinceEpoch(long long)
(qdatetime.cpp:3435)
==24943==by 0x573EB83: QDateTime::fromMSecsSinceEpoch(long long,
Qt::TimeSpec, int) (qdatetime.cpp:4266)
==24943==by 0x573EA8C: QDateTime::fromTime_t(unsigned int)
(qdatetime.cpp:4186)
==24943==by 0x5815ABB: QFileSystemMetaData::modificationTime() const
(qfilesystemmetadata_p.h:274)
==24943== Address 0x67a5ac0 is 0 bytes inside data symbol "timezone"
Should we have a QFileInfo::lastModifiedUtc()? Or to make this all much more
intuitive,
should there be a mutex in epochMSecsToLocalTime and localMSecsToEpochMSecs?
[the same one, obviously]
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
Re: KTabWidget vs QTabWidget
On Friday 18 September 2015 18:46:24 Jeremy Whiting wrote: > Hey all, > > In looking into fixing the remaining issues in Okular's frameworks > branch I realized that in part of the effort to port it away from > KDELibs4Support it got some functionality removed. It was ported from > KTabWidget to QTabWidget but QTabWidget doesn't seem to support drag > and drop the way KTabWidget did. In looking at the KTabWidget > documentation on api.kde.org it still says that KTabWidget is > preferred over QTabWidget [1]. If that's the case why did it end up in > KDELibs4Support? Nobody took the time to update the documentation of all the classes that were moved to kdelibs4support. So as always: trust the code more than the documentation. > In reading Qt documentation about drag and drop [2] it seems that you > need to subclass widgets in order to specify any additional mime types > that should be handled by a drop event (which okular made use of so > you could drop documents on it's tab bar to open them). Without > KTabWidget we lose that feature completely unless we subclass > QTabWidget (which we have in KTabWidget... so why not just use it...). > Am I missing something? If not I suggest we reconsider and maybe > move/copy? KTabWidget into KF5::WidgetsAddons as it still provides > functionality we want/need in some cases. I'm not sure what would be > BC or SC in this case tbh (or maybe users of KTabWidget should just > keep using KDELibs4Support?) To be clear: QTabWidget supports dnd-to-reorder tabs, but that's not what you're after, you want DnD of URLs, right? It takes about 10 lines of code in a QTabWidget subclass, but of course I have no objection to this being provided by KF5 for convenience. However, *not* as a QTabWidget subclass then. We moved away from that design because it creates confusion (when to use QTabWidget and when to use KTabWidget) and makes things less flexible. Instead, I suggest that you want a decorator, much like KDragWidgetDecorator in kwidgetaddons, but for drops of URLs. The decorator would install event filters for DragEnter and Drop, check for URLs, decode on drop, emit a signal. The only problem is: how would we tell the decorator that we only want drops "in the empty area next to the tabbar" and not everywhere on the tabwidget... (I assume this is what you want, right?). (I'm keeping the CC to kde-core-devel so its readers can see this has been answered, but I suggest we now followup on kde-frameworks-devel only). -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 125367: Make kfileplacesmodeltest pass
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125367/#review85858 --- Ship it! Ship It! - David Faure On Sept. 23, 2015, 11:34 p.m., Albert Astals Cid wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125367/ > --- > > (Updated Sept. 23, 2015, 11:34 p.m.) > > > Review request for kdelibs and David Faure. > > > Repository: kdelibs > > > Description > --- > > Bring back parent dir creation from KF5 > > Use the user-places.xbel file instead of bookmarks.xml > > > Diffs > - > > kfile/tests/kfileplacesmodeltest.cpp 834a605 > kio/bookmarks/kbookmarkmanager.cc 940cffd > > Diff: https://git.reviewboard.kde.org/r/125367/diff/ > > > Testing > --- > > Test passes now. > > > Thanks, > > Albert Astals Cid > >
Re: Naming scheme for Qt5/KF5-based libraries outside of KF5
On Monday 28 September 2015 01:43:59 Alexander Potashev wrote: > And you can also look at the numbers: KF5 grows at the rate of less > than one framework per month. We probably don't have enough manpower > to review dozens/hundreds of libraries in a short period of time. So you would rather aim for releasing crap ASAP than for quality? ;) On Sunday 27 September 2015 04:01:26 Boudhayan Gupta wrote: > We could kill two birds with one stone here, creating a new KDE module > just for libraries (say, KDE Companion Libraries or something) It seems to me that you're reinventing KF5. Either a lib is meant to be used by both apps and workspace (and possibly external apps) (see below for details), Or a lib is only meant to share code between apps (e.g. libkdegames) and it should be released as part of the apps. We can invent a new prefix or suffix for these (K5A?) but the main thing is that there is no need for a new product, for libs that are only used by the Applications product. If a lib is to be used by both apps and workspace, then I see two cases 1) the lib has stable API/ABI, then it's easy (name it -qt5 if you release it separately, like e.g. QCA, or make it a framework and it'll be released automatically) 2) the lib cannot promise ABI stability yet. In that case we can introduce another type of framework, let's say experimental to reuse an old kdelibs concept. [I suggested something like that in the "Baloo Framework - License Exception" thread 8 months ago, but that was not a solution back then (the license problem was too big). It would be one here, though, IMHO.] In practice my suggestion is: * just like we have "portingAids: true" in framework YAML files, we can have "experimental: true". * the tarballs for experimental frameworks go into a separate subdir (just like portingAids) * there would be no api docs on api.kde.org for these frameworks (otherwise external application developers would think they promise BIC just like proper KF5 does) * the .so number still has to go up every month where a BIC happens (that's the rule) * SIC should not happen. Since KF5, apps and workspace are not released together, a SIC would mean that upgrading one would break the other. One would have to make sure to keep deprecated stuff around, and to preserve behaviour. This is the price to pay for sharing between apps and workspace. Naming: "experimental" sounds like "it will be a framework at some point, when it stabilizes". Maybe we should say "internal" either, i.e. used internally by apps and workspace, don't use outside of that. Which still doesn't prevent a framework tagged "internal" from turning a proper public framework later. Maybe the "experimental" or "internal" should even be part of the tarball name and distro package names, to make really sure app developers see that. Again, none of this is needed for libs that are only for apps, or only for workspace. In summary, I see 6 types of libs: * public, separate (qca, poppler, libkolab, etc.) (external, or at least separately released) * public, part of KF5 (that's KF5 as you know it) * internal, part of KF5 (doesn't exist yet, but I'm open to the idea, see above) * internal, part of apps (e.g. libkdegames) * internal, part of workspace (e.g. libksysguard) * private to one app (e.g. libkonqueror_private, that's just for unittests) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 125476: Fix QUrl usage when calling QFileDialog::getExistingDirectory()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125476/#review86216 --- Ship it! Looks good, except for a bug in the commit log. You meant "should have been toLocalFile()" instead of "should have been path()" ;) - David Faure On Oct. 1, 2015, 8:58 p.m., Sergio Martins wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125476/ > --- > > (Updated Oct. 1, 2015, 8:58 p.m.) > > > Review request for kdelibs and David Faure. > > > Repository: kio > > > Description > --- > > - Passing openUrl.toString() is wrong, it should have been openUrl.path(), > since the former has a scheme, and QFileDialog will call > QUrl::fromLocalFile() on it. > - QFileDialog::getExistingDirectory() internally calls > QFileDialog::getExistingDirectoryUrl() and converts the result to a local > file string, which we then were re-converting to QUrl again, so instead just > call getExistingDirectoryUrl() directly. > > > Diffs > - > > src/widgets/kurlrequester.cpp 1b3bbdf > > Diff: https://git.reviewboard.kde.org/r/125476/diff/ > > > Testing > --- > > > Thanks, > > Sergio Martins > >
Re: Review Request 125476: Fix QUrl usage when calling QFileDialog::getExistingDirectory()
> On Oct. 2, 2015, 7:32 a.m., David Faure wrote: > > Looks good, except for a bug in the commit log. You meant "should have been > > toLocalFile()" instead of "should have been path()" ;) > > Sergio Martins wrote: > which return the same thing for local files, innit ? No it doesn't, not on Windows. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125476/#review86216 --- On Oct. 2, 2015, 3:19 p.m., Sergio Martins wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125476/ > --- > > (Updated Oct. 2, 2015, 3:19 p.m.) > > > Review request for kdelibs and David Faure. > > > Repository: kio > > > Description > --- > > - Passing openUrl.toString() is wrong, it should have been openUrl.path(), > since the former has a scheme, and QFileDialog will call > QUrl::fromLocalFile() on it. > - QFileDialog::getExistingDirectory() internally calls > QFileDialog::getExistingDirectoryUrl() and converts the result to a local > file string, which we then were re-converting to QUrl again, so instead just > call getExistingDirectoryUrl() directly. > > > Diffs > - > > src/widgets/kurlrequester.cpp 1b3bbdf > > Diff: https://git.reviewboard.kde.org/r/125476/diff/ > > > Testing > --- > > > Thanks, > > Sergio Martins > >
Re: Review Request 119892: [Dolphin] Implement "Add to archive" option when dragging and dropping onto an archive file in dolphin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119892/#review86318 --- lib/konq/konq_operations.cpp (line 441) <https://git.reviewboard.kde.org/r/119892/#comment59466> Yes, dropping a file onto an executable sounds like a good feature to have. Why disable it? It seems unrelated to the feature in this patch; just select the correct code path depending on whether you're dropping onto an executable or an archive Basically, refactor the code in the next hunk, which finds out if ark supports that mimetype, into a separate function that returns true if supported by ark, false if not. Doing the KService lookup twice is not a problem. - David Faure On Sept. 4, 2014, 7:20 a.m., Arjun AK wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119892/ > --- > > (Updated Sept. 4, 2014, 7:20 a.m.) > > > Review request for KDE Base Apps, KDE Utils and David Faure. > > > Bugs: 338414 > http://bugs.kde.org/show_bug.cgi?id=338414 > > > Repository: kde-baseapps > > > Description > --- > > This patch implements the "Add to archive" option, which is shown when a user > drags and drops files onto an existing archive. > > > See also: > https://git.reviewboard.kde.org/r/119890 > https://bugs.kde.org/show_bug.cgi?id=338414 > > > Diffs > - > > dolphin/src/views/dolphinview.cpp 02b8815 > dolphin/src/views/draganddrophelper.cpp f8ae0ad > lib/konq/konq_operations.cpp 220a90a > > Diff: https://git.reviewboard.kde.org/r/119892/diff/ > > > Testing > --- > > > Thanks, > > Arjun AK > >
Re: Review Request 119890: [Ark] Implement "Add to archive" option when dragging and dropping onto an archive file in dolphin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119890/#review86319 --- (Seems we don't have an ark maintainer...) This looks good to me. At this point it should be ported to KF5 though, I suppose. - David Faure On Aug. 21, 2014, 9:11 p.m., Arjun AK wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119890/ > --- > > (Updated Aug. 21, 2014, 9:11 p.m.) > > > Review request for KDE Base Apps and KDE Utils. > > > Bugs: 338414 > http://bugs.kde.org/show_bug.cgi?id=338414 > > > Repository: ark > > > Description > --- > > This patch implements the "Add to archive" option, which is shown when a user > drags and drops files onto an existing archive. > > > See also: > https://git.reviewboard.kde.org/r/119892 > https://bugs.kde.org/show_bug.cgi?id=338414 > > > Diffs > - > > app/ark_dndaddtoarchive.desktop.cmake PRE-CREATION > app/CMakeLists.txt f1ef01b > app/addToArchiveDndPlugin.h PRE-CREATION > app/addToArchiveDndPlugin.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/119890/diff/ > > > Testing > --- > > > Thanks, > > Arjun AK > >
Re: Review Request 125560: Give unique names to the targets created by KDE4Macros.cmake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125560/#review86607 --- Ship it! Ship It! - David Faure On Oct. 8, 2015, 5:38 p.m., Hrvoje Senjan wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125560/ > --- > > (Updated Oct. 8, 2015, 5:38 p.m.) > > > Review request for Build System, kdelibs and Luigi Toscano. > > > Repository: kdelibs > > > Description > --- > > Backport of review 116650. > Needed to fix build of some software after kdelibs' 4.14.11 cmake refactoring > > > Diffs > - > > cmake/modules/KDE4Macros.cmake 8622959 > > Diff: https://git.reviewboard.kde.org/r/125560/diff/ > > > Testing > --- > > k3b and skanlite tarballs now build. > > > Thanks, > > Hrvoje Senjan > >
KDE Frameworks 5.15.0 released
to import/export shortcut schemes symmetrically NetworkManagerQt Fix introspections, LastSeen should be in AccessPoint and not in ActiveConnection Plasma Framework Make tooltip dialog hidden on the cursor entering the inactive ToolTipArea if the desktop file has Icon=/foo.svgz use that file from package add a "screenshot" file type in packages consider devicepixelration in standalone scrollbar no hover effect on touchscreen+mobile Use lineedit svg margins in sizeHint calculation Don't fade animate icon in plasma tooltips Fix eliding button text Context menus of applets within a panel no longer overlap the applet Simplify getting associated apps list in AssociatedApplicationManager Sonnet Fix hunspell plugin ID for proper loading support static compilation on windows, add windows libreoffice hunspell dict path Do not assume UTF-8 encoded Hunspell dictionaries. (bug 353133) fix Highlighter::setCurrentLanguage() for the case when previous language was invalid (bug 349151) support /usr/share/hunspell as dict location NSSpellChecker-based plugin http://kde.org/announcements/kde-frameworks-5.15.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Naming scheme for Qt5/KF5-based libraries outside of KF5
On Saturday 17 October 2015 17:53:25 Alexander Potashev wrote: > Hi David, > > 2015-09-28 23:21 GMT+03:00 David Faure : > > Naming: "experimental" sounds like "it will be a framework at some point, > > when it > > stabilizes". Maybe we should say "internal" instead [was: either], i.e. > > used internally by apps > > and workspace, don't use outside of that. Which still doesn't prevent a > > framework > > tagged "internal" from turning a proper public framework later. > > > > Maybe the "experimental" or "internal" should even be part of the tarball > > name > > and distro package names, to make really sure app developers see that. > > Sounds good, I'm only worried about users/developers disregarding > these libraries because they have "experimental" in their names, even > though they may have been around and work OK for 5+ years already. Wait, if they have been around and work OK for 5+ years, isn't it time to stabilize and guarantee API and ABI? The whole point of "experimental" is to convey the message that the API isn't stable yet, so yes, the point *is* that developers should disregard such libraries if they need a stable API and ABI. If the point isn't that it will stabilize at some point (and become a real framework) then you want "internal" instead, as I said in the quoted text above. > We still need some distinct naming scheme/namespace for kf5-experimental, > right? Actually I don't think so. "experimental" becomes stable at some point, and at that point we don't want to have to port all users of the code. On the other hand, "internal" probably stays "internal" for ever, so for these it would make sense to have that in the library name maybe? Or just no KF5 in the name. Let me expand my summary a little bit, because I'm not sure which type of lib you're asking about exactly. I see 7 types of libs: * public, separate (qca, poppler, libkolab, etc.) (external, or at least separately released; no KF5 in the name) * public, part of KF5 (that's KF5 as you know it; API/ABI is guaranteed) * public but "experimental", released with KF5 (i.e. a framework that will stabilize and become part of the above) These mean "you can start using them now but ABI will break, or you can wait and you'll get stable ABI". kdelibs/kactivities was experimental like that AFAIK. * internal, part of KF5 (i.e. a lib meant to be used by apps and workspace, but not outside of that) To the outside world this says "do not use, ever" (or convince us to make it KF5 proper). * apps-internal (e.g. libkdegames; no KF5 in the name; cannot use in worskpace) * workspace-internal (e.g. libksysguard; no KF5 in the name; cannot use in apps) * private to one app (e.g. libkonqueror_private, that's just for unittests) Again, note that SIC are not possible anyway, for experimental or internal-in-KF5 libraries. So e.g. one can add virtual methods (and bump the so version), but not remove/rename anything, because of the separate release schedule for frameworks, apps and workspace. So is this really worth the separate categorization? If libs that are meant to be shared between apps and workspace need stable API anyway then they might as well become proper KF5 libs, being able to make BICs but not SICs is only a tiny gain IMHO. I suppose version-number-ifdefs in apps allow to make a few SICs too, but this is not trivial to do right (when apps code is already released and must keep compiling). Before going further about naming, please tell me which type of lib you're thinking about, and consider whether there is that much to be gained by using the "experimental" or "internal" concept for it. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 125613: Race condition and error notification loss in ListJob
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review87048 --- Thanks for the investigation and patch. kio/kio/copyjob.cpp (line 579) <https://git.reviewboard.kde.org/r/125613/#comment59824> We don't want to abort the copy just because one unreadable folder was found. We want to tell the user (hence the warning line below) and keep going. This emulates what `cp -a ~/src ~/dst` does. OK, you're not exactly aborting, but you're still setting an error - even though everything else got copied. I don't see why this is necessary. Is it just to get an error box, or does it fix behaviour? kio/kio/copyjob.cpp (line 582) <https://git.reviewboard.kde.org/r/125613/#comment59830> This is what is supposed to show you a message box about the fact that a subdirectory was skipped. If it doesn't work, I would suggest debugging that (maybe a problem with the default ui delegate). kio/kio/job.cpp (line 2659) <https://git.reviewboard.kde.org/r/125613/#comment59829> this makes sense indeed. kio/kio/job.cpp (line 2669) <https://git.reviewboard.kde.org/r/125613/#comment59828> isn't this the error message you're getting? I'm confused then (you're adding code that isn't getting called?) kio/kio/job.cpp (line 2671) <https://git.reviewboard.kde.org/r/125613/#comment59833> Same question here. As the comment says, "the result is still OK", which is why I didn't set an error code here. Please explain why this is needed. kio/kio/job.cpp (line 2683) <https://git.reviewboard.kde.org/r/125613/#comment59832> I see. SimpleJob::slotFinished() will emit result again, if it happens after this. This check is unusual, but then again, most other composite jobs do not also run a slave themselves. Can you add a comment? Like // if the main directory listing is still running, it will emit result in SimpleJob::slotFinished() kio/tests/jobtest.cpp (line 377) <https://git.reviewboard.kde.org/r/125613/#comment59825> Just call QDir().remove always, you don't check its return value anyway ;) kio/tests/jobtest.cpp (line 399) <https://git.reviewboard.kde.org/r/125613/#comment59826> Typo: premissions -> permissions kio/tests/jobtest.cpp (line 411) <https://git.reviewboard.kde.org/r/125613/#comment59827> please remove trailing space everywhere - David Faure On Oct. 17, 2015, 2:02 p.m., Alberto Jiménez Ruiz wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125613/ > --- > > (Updated Oct. 17, 2015, 2:02 p.m.) > > > Review request for kdelibs, Albert Astals Cid and David Faure. > > > Bugs: 333436 > http://bugs.kde.org/show_bug.cgi?id=333436 > > > Repository: kdelibs > > > Description > --- > > Race condition and error notification loss in ListJob > > > Diffs > - > > kio/kio/copyjob.cpp e6c3817 > kio/kio/job.cpp 91712e3 > kio/kio/jobclasses.h d771cfe > kio/tests/jobtest.h d3c552e > kio/tests/jobtest.cpp ee2677a > > Diff: https://git.reviewboard.kde.org/r/125613/diff/ > > > Testing > --- > > Update 1, added unittest > Changed condition of listjob slotresult finishing (When executed as a > synchronous job, gets stuck in a eventloop) > Added setError to copyjob. > > > Tests done on Kubuntu 15.10: > > With this folder structure in ~: > src > ##c (Folder) > ##b (Folder chmod'ed to 700 and owned by root) > ###d (10MB file made of /dev/urandom data) > ##a (10MB file made of /dev/urandom data) > > Then, (as a normal user execute: "kioclient ~/src ~/dst") > > Expected result: Notification that some files were not able to be copied > (Cannot access "b" folder) > Result with latest kdelibs: Silent copying where b folder is owned by user > but nothing inside. > ---When all kdebugdialog flags are set, a backtrace is also seen. > > subError notifications for listjob subjobs are lost, Copyjob uses this signal > to skip data. > > Also, this fix prevents a race condition. > > ListJob has another ListJob as a subjob. > > Chaild fails for whatever reason and calls error, which calls slotError, > which calls slotsFinished and emitResult. > The result of the child gets collected by parent. slotResult of parent gets > called, removes subjob and emitsresult because there are no subjobs left. > ---That's fine as long as the slave associated with the parent emits finished > before the child fails. If it doesn't, parent calls emitsResult twice. > > > Result after patch: kioclient shows a MessageBox telling the copy operation > could not be completed. > > > Thanks, > > Alberto Jiménez Ruiz > >
Re: Review Request 125613: Race condition and error notification loss in ListJob
> On Oct. 19, 2015, 7:28 a.m., David Faure wrote: > > kio/kio/job.cpp, line 2671 > > <https://git.reviewboard.kde.org/r/125613/diff/3/?file=411228#file411228line2671> > > > > Same question here. As the comment says, "the result is still OK", > > which is why I didn't set an error code here. Please explain why this is > > needed. > > Alberto Jiménez Ruiz wrote: > The comment exactly says: (If we can't list a subdir, the result is still > ok). > > I interpret it like the result is not ok if the error is not about being > unable to list a directory. Which other type of error would there be, when trying to list a subdirectory? Whatever the error is, the result is that we were not able to list that directory. (whether it's because of permission errors, or because it was deleted during the listing by the user (race condition), or ... not sure what else could happen, really) I think there's no reason for testing for a specific error code, and I also think we don't want to setError, since "it's still OK". - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review87048 --- On Oct. 17, 2015, 2:02 p.m., Alberto Jiménez Ruiz wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125613/ > ------- > > (Updated Oct. 17, 2015, 2:02 p.m.) > > > Review request for kdelibs, Albert Astals Cid and David Faure. > > > Bugs: 333436 > http://bugs.kde.org/show_bug.cgi?id=333436 > > > Repository: kdelibs > > > Description > --- > > Race condition and error notification loss in ListJob > > > Diffs > - > > kio/kio/copyjob.cpp e6c3817 > kio/kio/job.cpp 91712e3 > kio/kio/jobclasses.h d771cfe > kio/tests/jobtest.h d3c552e > kio/tests/jobtest.cpp ee2677a > > Diff: https://git.reviewboard.kde.org/r/125613/diff/ > > > Testing > --- > > Update 1, added unittest > Changed condition of listjob slotresult finishing (When executed as a > synchronous job, gets stuck in a eventloop) > Added setError to copyjob. > > > Tests done on Kubuntu 15.10: > > With this folder structure in ~: > src > ##c (Folder) > ##b (Folder chmod'ed to 700 and owned by root) > ###d (10MB file made of /dev/urandom data) > ##a (10MB file made of /dev/urandom data) > > Then, (as a normal user execute: "kioclient ~/src ~/dst") > > Expected result: Notification that some files were not able to be copied > (Cannot access "b" folder) > Result with latest kdelibs: Silent copying where b folder is owned by user > but nothing inside. > ---When all kdebugdialog flags are set, a backtrace is also seen. > > subError notifications for listjob subjobs are lost, Copyjob uses this signal > to skip data. > > Also, this fix prevents a race condition. > > ListJob has another ListJob as a subjob. > > Chaild fails for whatever reason and calls error, which calls slotError, > which calls slotsFinished and emitResult. > The result of the child gets collected by parent. slotResult of parent gets > called, removes subjob and emitsresult because there are no subjobs left. > ---That's fine as long as the slave associated with the parent emits finished > before the child fails. If it doesn't, parent calls emitsResult twice. > > > Result after patch: kioclient shows a MessageBox telling the copy operation > could not be completed. > > > Thanks, > > Alberto Jiménez Ruiz > >
Re: Review Request 125613: Race condition and error notification loss in ListJob
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review87242 --- Ship it! The fix looks fine to me now, thanks. The unittest can be improved a bit, feel free to commit after making the changes suggested below (or tell me if you have no account) kio/tests/jobtest.cpp (line 374) <https://git.reviewboard.kde.org/r/125613/#comment59923> missing spaces around '=' sign kio/tests/jobtest.cpp (line 395) <https://git.reviewboard.kde.org/r/125613/#comment59924> bitfield operations on booleans are not a good idea. I wish there was a &&=, like you but (at least in C++98) there isn't, so the only solution is ok_copyjob = ok_copyjob && ... But anyway the whole idea of using "and" means that on failure we don't know which condition failed. Why not QVERIFY() both statements separately? kio/tests/jobtest.cpp (line 413) <https://git.reviewboard.kde.org/r/125613/#comment59925> alternatively, you could use QSignalSpy, then you don't need a slot :-) - David Faure On Oct. 19, 2015, 9:43 a.m., Alberto Jiménez Ruiz wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125613/ > --- > > (Updated Oct. 19, 2015, 9:43 a.m.) > > > Review request for kdelibs, Albert Astals Cid and David Faure. > > > Bugs: 333436 > http://bugs.kde.org/show_bug.cgi?id=333436 > > > Repository: kdelibs > > > Description > --- > > Race condition and error notification loss in ListJob > > > Diffs > - > > kio/kio/job.cpp 91712e3 > kio/kio/jobclasses.h d771cfe > kio/tests/jobtest.h d3c552e > kio/tests/jobtest.cpp ee2677a > > Diff: https://git.reviewboard.kde.org/r/125613/diff/ > > > Testing > --- > > Update 1, added unittest > Changed condition of listjob slotresult finishing (When executed as a > synchronous job, gets stuck in a eventloop) > Added setError to copyjob. > > > Tests done on Kubuntu 15.10: > > With this folder structure in ~: > src > ##c (Folder) > ##b (Folder chmod'ed to 700 and owned by root) > ###d (10MB file made of /dev/urandom data) > ##a (10MB file made of /dev/urandom data) > > Then, (as a normal user execute: "kioclient ~/src ~/dst") > > Expected result: Notification that some files were not able to be copied > (Cannot access "b" folder) > Result with latest kdelibs: Silent copying where b folder is owned by user > but nothing inside. > ---When all kdebugdialog flags are set, a backtrace is also seen. > > subError notifications for listjob subjobs are lost, Copyjob uses this signal > to skip data. > > Also, this fix prevents a race condition. > > ListJob has another ListJob as a subjob. > > Chaild fails for whatever reason and calls error, which calls slotError, > which calls slotsFinished and emitResult. > The result of the child gets collected by parent. slotResult of parent gets > called, removes subjob and emitsresult because there are no subjobs left. > ---That's fine as long as the slave associated with the parent emits finished > before the child fails. If it doesn't, parent calls emitsResult twice. > > > Result after patch: kioclient shows a MessageBox telling the copy operation > could not be completed. > > > Thanks, > > Alberto Jiménez Ruiz > >
Re: Review Request 125613: Race condition and error notification loss in ListJob
f372 in QAbstractButton::clicked
> > (this=this@entry=0x8c1fc0, _t1=false) at .moc/moc_qabstractbutton.cpp:303
> > #34 0x73b8e654 in QAbstractButtonPrivate::emitClicked
> > (this=0x8c27e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/widgets/qabstractbutton.cpp:533
> > #35 0x73b8fb29 in QAbstractButtonPrivate::click (this=0x8c27e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/widgets/qabstractbutton.cpp:526
> > #36 0x73b8fc7c in QAbstractButton::mouseReleaseEvent
> > (this=0x8c1fc0, e=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/widgets/qabstractbutton.cpp:1131
> > #37 0x73af3588 in QWidget::event (this=0x8c1fc0,
> > event=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qwidget.cpp:9064
> > #38 0x73ab2dcc in QApplicationPrivate::notify_helper
> > (this=, receiver=0x8c1fc0, e=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3717
> > #39 0x73ab88ce in QApplication::notify (this=,
> > receiver=0x8c1fc0, e=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3275
> > #40 0x7311d2d8 in QCoreApplication::notifyInternal2
> > (receiver=receiver@entry=0x8c1fc0, event=event@entry=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/corelib/kernel/qcoreapplication.cpp:1002
> > #41 0x73ab745e in QCoreApplication::sendSpontaneousEvent
> > (event=0x7fffd4e0, receiver=0x8c1fc0) at
> > ../../include/QtCore/../../../../frameworks/qt5/qtbase/src/corelib/kernel/qcoreapplication.h:230
> > #42 QApplicationPrivate::sendMouseEvent (receiver=receiver@entry=0x8c1fc0,
> > event=event@entry=0x7fffd4e0, alienWidget=alienWidget@entry=0x8c1fc0,
> > nativeWidget=0x7fffdce0, buttonDown=buttonDown@entry=0x73fa4ba0
> > , lastMouseReceiver=...,
> > spontaneous=true) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:2770
> > #43 0x73b0daa1 in QWidgetWindow::handleMouseEvent
> > (this=this@entry=0x7be360, event=event@entry=0x7fffd8f0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qwidgetwindow.cpp:554
> > #44 0x73b0fe7b in QWidgetWindow::event (this=0x7be360,
> > event=0x7fffd8f0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qwidgetwindow.cpp:210
> > #45 0x73ab2dcc in QApplicationPrivate::notify_helper
> > (this=, receiver=0x7be360, e=0x7fffd8f0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3717
> > #46 0x73ab7d96 in QApplication::notify (this=0x7fffde80,
> > receiver=0x7be360, e=0x7fffd8f0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3498
> > ```
>
> Alberto Jiménez Ruiz wrote:
> Can't reproduce on Kubuntu 15.10.
>
> Thomas Lübking wrote:
> Smells like a corrupted stack or threading issue.
>
> if (dirData.listersCurrentlyListing.isEmpty()) {
> qWarning() << "PS, nothing in
> directoryData.listersCurrentlyListing for" << jobUrlStr;
> // We're about to assert; dump the current state...
> #ifndef NDEBUG
> printDebug();
> #endif
> Q_ASSERT(!dirData.listersCurrentlyListing.isEmpty());
> }
>
> unless printDebug() is capable of polluting
> dirData.listersCurrentlyListing (it actually looks like that, but you must
> undefine NDEBUG), either the stack is junk or dirData.listersCurrentlyListing
> got some elements while we were printing the debug info from some other
> thread.
>
> I'm no way sure what that code is supposed to do but testing
> if (b)
>Q_ASSERT(!b);
> looks insane.
>
> In doubt there should be some mutex somewhere or the entire thing doesn't
> make much sense.
>
> It would seem the stricter test to emitResult() causes a later emit and
> ultimately triggers this.
> kate will be KF5 - and you need to build debug enabled to be Q_ASSERT !=
> NOOP (so just watch out whether you get the warning)
>
> Aleix, if you can reproduce this at will, I suggest to check whether
> indeed at some point ::printDebug() manipulates
> dirData.listersCurrentlyListing - eg. add a global reference and test it in
> each loop in ::printDebug()
if (b) {
// some debug output
Q_ASSERT(!b);
}
is definitely not insane, it's the way to print out more information about the
proble
Re: Review Request 125613: Race condition and error notification loss in ListJob
f372 in QAbstractButton::clicked
> > (this=this@entry=0x8c1fc0, _t1=false) at .moc/moc_qabstractbutton.cpp:303
> > #34 0x73b8e654 in QAbstractButtonPrivate::emitClicked
> > (this=0x8c27e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/widgets/qabstractbutton.cpp:533
> > #35 0x73b8fb29 in QAbstractButtonPrivate::click (this=0x8c27e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/widgets/qabstractbutton.cpp:526
> > #36 0x73b8fc7c in QAbstractButton::mouseReleaseEvent
> > (this=0x8c1fc0, e=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/widgets/qabstractbutton.cpp:1131
> > #37 0x73af3588 in QWidget::event (this=0x8c1fc0,
> > event=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qwidget.cpp:9064
> > #38 0x73ab2dcc in QApplicationPrivate::notify_helper
> > (this=, receiver=0x8c1fc0, e=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3717
> > #39 0x73ab88ce in QApplication::notify (this=,
> > receiver=0x8c1fc0, e=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3275
> > #40 0x7311d2d8 in QCoreApplication::notifyInternal2
> > (receiver=receiver@entry=0x8c1fc0, event=event@entry=0x7fffd4e0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/corelib/kernel/qcoreapplication.cpp:1002
> > #41 0x73ab745e in QCoreApplication::sendSpontaneousEvent
> > (event=0x7fffd4e0, receiver=0x8c1fc0) at
> > ../../include/QtCore/../../../../frameworks/qt5/qtbase/src/corelib/kernel/qcoreapplication.h:230
> > #42 QApplicationPrivate::sendMouseEvent (receiver=receiver@entry=0x8c1fc0,
> > event=event@entry=0x7fffd4e0, alienWidget=alienWidget@entry=0x8c1fc0,
> > nativeWidget=0x7fffdce0, buttonDown=buttonDown@entry=0x73fa4ba0
> > , lastMouseReceiver=...,
> > spontaneous=true) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:2770
> > #43 0x73b0daa1 in QWidgetWindow::handleMouseEvent
> > (this=this@entry=0x7be360, event=event@entry=0x7fffd8f0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qwidgetwindow.cpp:554
> > #44 0x73b0fe7b in QWidgetWindow::event (this=0x7be360,
> > event=0x7fffd8f0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qwidgetwindow.cpp:210
> > #45 0x73ab2dcc in QApplicationPrivate::notify_helper
> > (this=, receiver=0x7be360, e=0x7fffd8f0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3717
> > #46 0x73ab7d96 in QApplication::notify (this=0x7fffde80,
> > receiver=0x7be360, e=0x7fffd8f0) at
> > /home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3498
> > ```
>
> Alberto Jiménez Ruiz wrote:
> Can't reproduce on Kubuntu 15.10.
>
> Thomas Lübking wrote:
> Smells like a corrupted stack or threading issue.
>
> if (dirData.listersCurrentlyListing.isEmpty()) {
> qWarning() << "PS, nothing in
> directoryData.listersCurrentlyListing for" << jobUrlStr;
> // We're about to assert; dump the current state...
> #ifndef NDEBUG
> printDebug();
> #endif
> Q_ASSERT(!dirData.listersCurrentlyListing.isEmpty());
> }
>
> unless printDebug() is capable of polluting
> dirData.listersCurrentlyListing (it actually looks like that, but you must
> undefine NDEBUG), either the stack is junk or dirData.listersCurrentlyListing
> got some elements while we were printing the debug info from some other
> thread.
>
> I'm no way sure what that code is supposed to do but testing
> if (b)
>Q_ASSERT(!b);
> looks insane.
>
> In doubt there should be some mutex somewhere or the entire thing doesn't
> make much sense.
>
> It would seem the stricter test to emitResult() causes a later emit and
> ultimately triggers this.
> kate will be KF5 - and you need to build debug enabled to be Q_ASSERT !=
> NOOP (so just watch out whether you get the warning)
>
> Aleix, if you can reproduce this at will, I suggest to check whether
> indeed at some point ::printDebug() manipulates
> dirData.listersCurrentlyListing - eg. add a global reference and test it in
> each loop in ::printDebug()
>
> David Faure wrote:
> if (b) {
>// some debug output
>Q_ASSERT(!b);
> }
> is definitely not insa
Re: Review Request 125983: Give each htmlhandbook target a unique name
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125983/#review88155 --- Ship it! Ship It! - David Faure On Nov. 7, 2015, 1:59 p.m., Heiko Becker wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125983/ > --- > > (Updated Nov. 7, 2015, 1:59 p.m.) > > > Review request for Build System and kdelibs. > > > Bugs: 351287 > http://bugs.kde.org/show_bug.cgi?id=351287 > > > Repository: kdelibs > > > Description > --- > > Since the CMP0002 warnings have been turned into errors it was > impossible to build kdelibs with -DKDE4_ENABLE_HTMLHANDBOOK:BOOL=TRUE: > > "CMake Error at cmake/modules/KDE4Macros.cmake:315 (add_custom_target): > add_custom_target cannot create target "htmlhandbook" because another > target with the same name already exists" > > The bug below is more about the similar error with -DKDE4_BUILD_TESTS > but since it's still open (and I commented there about the htmlhandbook > error ;-) I included it here. > > BUG: 351287 > > > Diffs > - > > cmake/modules/KDE4Macros.cmake 5b3db07206d8075fdb580fda9f0b1f8d76a80511 > > Diff: https://git.reviewboard.kde.org/r/125983/diff/ > > > Testing > --- > > Passed -DKDE4_ENABLE_HTMLHANDBOOK:BOOL=TRUE to the build, which succeeded > withouth error. > > > Thanks, > > Heiko Becker > >
Re: Review Request 125995: Only install konqpopupmenuplugin.desktop in kde-baseapps if the KF5 version is less than 5.16
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125995/#review88169 --- Ship it! Ship It! - David Faure On Nov. 8, 2015, 6:45 p.m., Frank Reininghaus wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125995/ > --- > > (Updated Nov. 8, 2015, 6:45 p.m.) > > > Review request for KDE Base Apps and David Faure. > > > Repository: kde-baseapps > > > Description > --- > > KIO installs this file since version 5.16. > > Even if the frameworks branch of lib/konq is currently unreleased, resolving > this file installation conflict makes life easier for those who package it > (reported in https://bugs.kde.org/show_bug.cgi?id=353642#c12 ) > > > Diffs > - > > lib/konq/src/CMakeLists.txt b2d0c7e > > Diff: https://git.reviewboard.kde.org/r/125995/diff/ > > > Testing > --- > > kde-baseapps does not try to install the file any more. If I artificially > increase the checked version to 5.17, it does try, so I'm confident that this > will work fine with KIO versions prior to 5.16 and install the file (which is > needed to make the context menu for files work properly in Dolphin, > FolderView, Konqueror, etc.). > > > Thanks, > > Frank Reininghaus > >
KDE Frameworks 5.16.0 released
osing parentheses under some conditions Fix shortcutoverride not being forwarded to the mainwindow Bug 342659 - Default "bracket highlighting" color is hard to see (Normal schema fixed) (bug 342659) Add proper default colors for "Current Line Number" color bracket matching & auto-brackets: share code bracket matching: guard against negative maxLines bracket matching: just because the new range matches the old doesn't mean no update is required Add the width of half a space to allow painting the cursor at EOL fix some HiDPI issues in the icon border fix bug #310712: remove trailing spaces also on line with cursor (bug 310712) only display "mark set" message when vi input mode is active remove & from button text (bug 345937) fix update of current line number color (bug 340363) implement brackets insert on writing a bracket over a selection (bug 350317) auto brackets (bug 350317) fix alert HL (bug 32) no column scrolling with dyn word wrap on remember if highlighting was set by user over sessions to not loose it on save after restore (bug 332605) fix folding for tex (bug 328348) fixed bug #327842: End of C-style comment is misdetected (bug 327842) save/restore dyn word wrap on session save/restore (bug 284250) KTextWidgets Add a new submenu to KTextEdit to switch between spell-checking languages Fix loading Sonnet default settings KWallet Framework Use KDE_INSTALL_DBUSINTERFACEDIR to install dbus interfaces Fixed KWallet configuration file warnings on login (bug 351805) Prefix the kwallet-pam output properly KWidgetsAddons Add collapsible container widget, KCollapsibleGroupBox KNewPasswordWidget: missing color initialization Introduce KNewPasswordWidget KXMLGUI kmainwindow: Pre-fill translator information when available. (bug 345320) Allow to bind the contextmenu key (lower right) to shortcuts (bug 165542) Add function to query standards xml file location Allow kxmlgui framework to be used without any installed files Add missing required dependencies Plasma Framework Fix TabBar items being cramped together on initial creation, which can be observed in eg. Kickoff after Plasma start Fix dropping files onto the desktop/panel not offering a selection of actions to take Take QApplication::wheelScrollLines into account from ScrollView Use BypassWindowManagerHint only on platform X11 delete old panel background more readable spinner at small sizes colored view-history calendar: Make the entire header area clickable calendar: Don't use current day number in goToMonth calendar: Fix updating decade overview Theme breeze icons when loaded trough IconItem Fix Button minimumWidth property (bug 353584) Introduce appletCreated signal Plasma Breeze Icon: Touchpad add svg id elements Plasma Breeze Icon: change Touchpad to 22x22px size Breeze Icon: add widget icon to notes A script to replace hardcoded colors with stylesheets Apply SkipTaskbar on ExposeEvent Don't set SkipTaskbar on every event http://kde.org/announcements/kde-frameworks-5.16.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KDE Frameworks 5.10.0 released
[didn't see this mail until now, just no time to read 1819 emails on k-c-d] On Saturday 09 May 2015 12:31:14 Jan Kundrát wrote: > Hi David, > could you please clarify the release procedure, in particular what > determines whether commits pushed after the -rc1 tag are included or not? They are not, unless someone tells me to redo a tarball. > Should I send a mail to the release team next time, maybe? Yes. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KSyCoca, Thread safety, and Cache invalidation
On Monday 14 September 2015 20:49:26 Thiago Macieira wrote: > On Sunday 13 September 2015 23:04:01 David Faure wrote: > > On Tuesday 14 July 2015 16:01:09 Thiago Macieira wrote: > > > If you need a machine-comparable time with other systems or across > > > reboots, > > > use QDateTime::currentDateTimeUtc(). That avoids refreshing the timezone > > > database to convert to local time. > > > > > > Only use QDateTime::currentDateTime() if you want to show something to the > > > user. There's no other valid reason. (writing to a log counts as "showing > > > to the user") > > > > What if I need to compare a file's modification time with a given timestamp? > > File times are kept in UTC on disk. Wow, so we do quite some unnecessary roundtrips between UTC and localtime indeed. > > This code: > > > > qCDebug(SYCOCA) << "checking file timestamps"; > > const QDateTime stamp = QDateTime::fromMSecsSinceEpoch(timestamp); > > You're using the function that creates a LocalTime timestamp and yet: QDateTime::fromMSecsSinceEpoch() indeed creates a localtime timestamp, which means it calls qt_localtime() which calls tzset(), so it's not threadsafe. #1 0x77118d1f in qt_tzset () at tools/qdatetime.cpp:2117 #2 0x77119194 in qt_localtime (msecsSinceEpoch=1447506886000, localDate=0x7fffbed0, localTime=0x7fffbec0, daylightStatus=0x7fffbebc) at tools/qdatetime.cpp:2 333 #3 0x77119622 in epochMSecsToLocalTime (msecs=1447506886000, localDate=0x7fffbed0, localTime=0x7fffbec0, daylightStatus=0x7fffbebc) at tools/qdatetime.cpp:24 40 #4 0x7711aecb in QDateTime::setMSecsSinceEpoch (this=0x7fffc110, msecs=1447506886000) at tools/qdatetime.cpp:3436 #5 0x7711c876 in QDateTime::fromMSecsSinceEpoch (msecs=1447506886000, spec=Qt::LocalTime, offsetSeconds=0) at tools/qdatetime.cpp:4267 #6 0x7711c77f in QDateTime::fromTime_t (seconds=1447506886) at tools/qdatetime.cpp:4187 #7 0x771f529c in QFileSystemMetaData::modificationTime (this=0x62edf0) at io/qfilesystemmetadata_p.h:274 #8 0x771f4f5c in QFileInfo::lastModified (this=0x7fffc198) at io/qfileinfo.cpp:1336 Who would expect this to not be threadsafe? > > Should we have a QFileInfo::lastModifiedUtc()? Or to make this all much more > > intuitive, should there be a mutex in epochMSecsToLocalTime and > > localMSecsToEpochMSecs? [the same one, obviously] > > What if we changed the existing lastModified() to return UTC? As I said, the > date is stored in UTC already. > > Though I imagine quite a few places don't convert to local time and would > start showing UTC time unexpectedly... Yeah, I'm 100% sure most code out there doesn't convert to localtime, hence my suggestion of QFileInfo::lastModifiedUtc(), to avoid breaking code. But then we also need QDateTime::fromMSecsSinceEpoch(time_t, UTC) ? Is there really no way to do localtime/UTC conversions in a threadsafe way? This tzset() issue is really awful, we'll never manage to make sure that 100% of the code that needs to be threadsafe uses UTC everywhere. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
KDE Frameworks 5.17.0 released
12th December 2015. KDE today announces the release of KDE Frameworks 5.17.0. KDE Frameworks are 70 addon libraries to Qt which provide a wide variety of· commonly needed functionality in mature, peer reviewed and well tested· libraries with friendly licensing terms. For an introduction see the· Frameworks 5.0 release announcement. Baloo Fix date filter used by timeline:// BalooCtl: Return after commands Clean up and armour Baloo::Database::open(), handle more crash conditions Add check in Database::open(OpenDatabase) to fail if db doesn't exist Breeze Icons Many icons added or improved use stylesheets in breeze icons (bug 126166) BUG: 355902 fix and changed system-lock-screen (bug 355902 fix and changed system-lock-screen) Add 24px dialog-information for GTK apps (bug 355204) Extra CMake Modules Don't warn when SVG(Z) icons are provided with multiple sizes/level of detail Make sure we load translations on the main thread. (bug 346188) Overhaul the ECM build system. Make it possible to enable Clazy on any KDE project Do not find XCB's XINPUT library by default. Clean export dir before generating an APK again Use quickgit for Git repository URL. Framework Integration Add plasmoid installation failed to plasma_workspace.notifyrc KActivities Fixed a lock on the first start of the daemon Moving QAction creation to the main thread. (bug 351485) Sometimes clang-format makes a bad decision (bug 355495) Killing potential synchronization issues Use org.qtproject instead of com.trolltech Removing the usage of libkactivities from the plugins KAStats config removed from the API Added linking and unlinking to ResultModel KDE Doxygen Tools Make kgenframeworksapidox more robust. KArchive Fix KCompressionDevice::seek(), called when creating a KTar on top of a KCompressionDevice. KCoreAddons KAboutData: Allow https:// and other URL schemas in homepage. (bug 355508) Repair MimeType property when using kcoreaddons_desktop_to_json() KDeclarative Port KDeclarative to use KI18n directly DragArea delegateImage can now be a string from which an icon is automatically created Add new CalendarEvents library KDED Unset SESSION_MANAGER envvar instead of setting it empty KDELibs 4 Support Fix some i18n calls. KFileMetaData Mark m4a as readable by taglib KIO Cookie dialogue: make it work as intended Fix filename suggestion changing to something random when changing save-as mimetype. Register DBus name for kioexec (bug 353037) Update KProtocolManager after configuration change. KItemModels Fix KSelectionProxyModel usage in QTableView (bug 352369) Fix resetting or changing the source model of a KRecursiveFilterProxyModel. KNewStuff registerServicesByGroupingNames can define default more items Make KMoreToolsMenuFactory::createMenuFromGroupingNames lazy KTextEditor Add syntax highlighting for TaskJuggler and PL/I Make it possible to disable keyword-completion via the config interface. Resize the tree when the completion model got reset. KWallet Framework Correctly handle the case where the user deactivated us KWidgetsAddons Fix a small artifact of KRatingWidget on hi-dpi. Refactor and fix the feature introduced in bug 171343 (bug 171343) KXMLGUI Don't call QCoreApplication::setQuitLockEnabled(true) on init. Plasma Framework Add basic plasmoid as example for developerguide Add a couple of plasmoid templates for kapptemplate/kdevelop [calendar] Delay the model reset until the view is ready (bug 355943) Don't reposition while hiding. (bug 354352) [IconItem] Don't crash on null KIconLoader theme (bug 355577) Dropping image files onto a panel will no longer offer to set them as wallpaper for the panel Dropping a .plasmoid file onto a panel or the desktop will install and add it remove the now unused platformstatus kded module (bug 348840) allow paste on password fields fix positioning of edit menu, add a button to select [calendar] Use ui language for getting the month name (bug 353715) [calendar] Sort the events by their type too [calendar] Move the plugin library to KDeclarative [calendar] qmlRegisterUncreatableType needs a bit more arguments Allow adding config categories dynamically [calendar] Move the plugins handling to a separate class Allow plugins to supply event data to Calendar applet (bug 349676) check for slot existence before connecting or disconnecting (bug 354751) [plasmaquick] Don't link OpenGL explicitly [plasmaquick] Drop XCB::COMPOSITE and DAMAGE dependency http://kde.org/announcements/kde-frameworks-5.17.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126308/#review89737 --- src/kdeui/kpushbutton.cpp (line 256) <https://git.reviewboard.kde.org/r/126308/#comment61479> This patch looks wrong because KPushButton can be used outside of "dialog button boxes", while the styleHint is supposed to be only about dialog button boxes. QPushButton::sizeHint does this: bool showButtonBoxIcons = qobject_cast(parentWidget()) && style()->styleHint(QStyle::SH_DialogButtonBox_ButtonsHaveIcons); which is a solution for testing the parent widget. I still don't fully understand the issue though, at painting time both QPushButton and KPushButton call QStyle's CE_PushButton, so I don't see why these two would be working differently. Is this a workaround for KPushButton which doesn't fix QPushButton? - David Faure On Dec. 11, 2015, 4:26 p.m., René J.V. Bertin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126308/ > --- > > (Updated Dec. 11, 2015, 4:26 p.m.) > > > Review request for KDE Software on Mac OS X, KDE Frameworks, Qt KDE, Hugo > Pereira Da Costa, and Yichao Yu. > > > Repository: kdelibs4support > > > Description > --- > > KF5 applications have long had a habit of drawing icons on buttons even when > this feature was turned off in the user's setting. This was mostly noticeable > in applications built on kdelibs4support. > > It seems that the actual culprit is in Qt's QPushButton implementation > (https://bugreports.qt.io/browse/QTBUG-49887), but it is possible to work > around it in `KPushButton::paintEvent`, by removing the icon (forcing it to > the null icon) in the option instance, before handing off control to the > painter. > > > Diffs > - > > src/kdeui/kdialogbuttonbox.cpp 0f6649b > src/kdeui/kpushbutton.cpp 98534fa > > Diff: https://git.reviewboard.kde.org/r/126308/diff/ > > > Testing > --- > > On Kubuntu 14.04 and OS X 10.9.5 with Qt 5.5.1 and KF5 frameworks 5.16.0 . > > I have not yet verified if there are other classes where this modification > would be relevant too. > > > Thanks, > > René J.V. Bertin > >
Re: KF5_ADD_KDEINIT_EXECUTABLE linker errors
On Friday 01 January 2016 15:45:45 Martin Koller wrote:
> Trying to port cervisia to KF5, I struggle with above.
>
> Having
> kf5_add_kdeinit_executable(cvsaskpass cvsaskpass.cpp)
>
> always gives linker errors:
> CMakeFiles/cvsaskpass.dir/cvsaskpass_dummy.cpp.o: In function `kdeinitmain':
> cvsaskpass_dummy.cpp:(.text+0x1c): undefined reference to `kdemain'
> CMakeFiles/cvsaskpass.dir/cvsaskpass_dummy.cpp.o: In function `main':
> cvsaskpass_dummy.cpp:(.text+0x3e): undefined reference to `kdemain'
>
> checking the cmake macro in /usr/lib64/cmake/KF5Init/KF5InitMacros.cmake, I
> wonder why
> is this even there:
> add_executable(${_target_NAME}
> ${CMAKE_CURRENT_BINARY_DIR}/${_target_NAME}_dummy.cpp ${_resourcefile})
>
> and if it's needed, then why does it not add ${_SRCS} directly, since by
> default
> the visbility of symbols is set to hidden, so this
> target_link_libraries(${_target_NAME} kdeinit_${_target_NAME})
> can't work.
> cvsaskpass.cpp contains:
> extern "C" int kdemain(int argc, char** argv)...
>
> What am I missing ?
Q_DECL_EXPORT before int kdemain.
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
Re: Review Request 126253: kxmlgui.xsd add Separator append attribute
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126253/#review90469 --- Ship it! kpartgui.dtd is mostly used for documentation purposes. kxmlgui.xsd is "more recent" (relatively speaking) and can also be used for validating files (xsd is more powerful for this). Maybe we can kill the DTD file these days. - David Faure On Dec. 6, 2015, 2:34 p.m., Allen Winter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126253/ > --- > > (Updated Dec. 6, 2015, 2:34 p.m.) > > > Review request for kdelibs and Martin Walch. > > > Bugs: 356134 > https://bugs.kde.org/show_bug.cgi?id=356134 > > > Repository: kxmlgui > > > Description > --- > > Add an append attribute to the Separator Element of the kxmlgui.xsd > > > Diffs > - > > src/kxmlgui.xsd da1d155 > > Diff: https://git.reviewboard.kde.org/r/126253/diff/ > > > Testing > --- > > added to Krazy now. no longer see the xmllint warnings in kolf > > > Thanks, > > Allen Winter > >
Re: Review Request 126204: kdialog: Implement new password dialog
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126204/#review90470 --- Ship it! Ship It! - David Faure On Nov. 30, 2015, 9:27 a.m., Andrew Chen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126204/ > --- > > (Updated Nov. 30, 2015, 9:27 a.m.) > > > Review request for KDE Base Apps. > > > Bugs: 340397 > http://bugs.kde.org/show_bug.cgi?id=340397 > > > Repository: kde-baseapps > > > Description > --- > > Using KNewPasswordDialog > > Usage: --newpassword [min len] [max len] New Password dialog > > > Diffs > - > > kdialog/kdialog.cpp 9afc53b > kdialog/widgets.h 2afbed5 > kdialog/widgets.cpp c03cb5a > > Diff: https://git.reviewboard.kde.org/r/126204/diff/ > > > Testing > --- > > > Thanks, > > Andrew Chen > >
Re: Review Request 126659: [kio_ftp] fix display of file/directory modification time/date
> On Jan. 7, 2016, 11:15 a.m., David Faure wrote: > > src/ioslaves/ftp/ftp.cpp, line 1775 > > <https://git.reviewboard.kde.org/r/126659/diff/1/?file=428702#file428702line1775> > > > > The porting bug is here. In kdelibs4 tmptr was initialized to the > > current date, and used below. > > > > Now we have two variables, "int year" and "currentTime.year". > > > > To be closer to the orig code and to avoid risking that the day or > > month is still 0 as well, I would suggest to initialize year, day and month > > directly from currentTime, rather than to 0. > > > > I guess we can also remove the "currentTime.setTime(QTime(0,0,0)) > > because that's unused (right?) > > Wolfgang Bauer wrote: > > To be closer to the orig code and to avoid risking that the day or > month is still 0 as well, I would suggest to initialize year, day and month > directly from currentTime, rather than to 0. > > Well, if we don't know the values, taking them from currentTime is likely > just as wrong as setting them to 0. > But ok, I'll change it. > > > I guess we can also remove the "currentTime.setTime(QTime(0,0,0)) > because that's unused (right?) > > Right, it is unused AFAICS. > Actually we could also just use QDate::currentDate() instead of > QDateTime::currentDateTime() IMHO. "likely just as wrong" - well with 0 the QDateTime would be invalid, making the whole information disappear (even e.g. the valid time information). So using the current datetime (as kdelibs4 did) seems better. Agreed about currentDate(). - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126659/#review90743 --- On Jan. 7, 2016, 12:23 p.m., Wolfgang Bauer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126659/ > --- > > (Updated Jan. 7, 2016, 12:23 p.m.) > > > Review request for KDE Frameworks, kdelibs and David Faure. > > > Bugs: 354597 > https://bugs.kde.org/show_bug.cgi?id=354597 > > > Repository: kio > > > Description > --- > > - QDate() treats the year literally (i.e. 90 is really year 90, not 1990), so > subtracting 1900 is wrong. > - Use QDate::currentDate() instead of QDateTime::currentDateTime(), we only > need the current date anyway > - Initialize day, month, and year to the current date instead of 0. In the > case when no year is mentioned in the server's reply (the year is implicit), > it wasn't set to the current year at all, so the result was either 0 or -1. > > > Diffs > - > > src/ioslaves/ftp/ftp.cpp 2179179 > > Diff: https://git.reviewboard.kde.org/r/126659/diff/ > > > Testing > --- > > Connected to an FTP server with dolphin (15.12.0). The modification > times/dates are now shown correctly. > > > Thanks, > > Wolfgang Bauer > >
Re: Review Request 126659: [kio_ftp] fix display of file/directory modification time/date
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126659/#review90750 --- Ship it! Ship It! - David Faure On Jan. 7, 2016, 12:23 p.m., Wolfgang Bauer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126659/ > --- > > (Updated Jan. 7, 2016, 12:23 p.m.) > > > Review request for KDE Frameworks, kdelibs and David Faure. > > > Bugs: 354597 > https://bugs.kde.org/show_bug.cgi?id=354597 > > > Repository: kio > > > Description > --- > > - QDate() treats the year literally (i.e. 90 is really year 90, not 1990), so > subtracting 1900 is wrong. > - Use QDate::currentDate() instead of QDateTime::currentDateTime(), we only > need the current date anyway > - Initialize day, month, and year to the current date instead of 0. In the > case when no year is mentioned in the server's reply (the year is implicit), > it wasn't set to the current year at all, so the result was either 0 or -1. > > > Diffs > - > > src/ioslaves/ftp/ftp.cpp 2179179 > > Diff: https://git.reviewboard.kde.org/r/126659/diff/ > > > Testing > --- > > Connected to an FTP server with dolphin (15.12.0). The modification > times/dates are now shown correctly. > > > Thanks, > > Wolfgang Bauer > >
KDE Frameworks 5.18.0 released
09th January 2016. KDE today announces the release of KDE Frameworks 5.18.0. KDE Frameworks are 70 addon libraries to Qt which provide a wide variety of· commonly needed functionality in mature, peer reviewed and well tested· libraries with friendly licensing terms. For an introduction see the· Frameworks 5.0 release announcement. Baloo Fix several issue of mtime related search PostingDB Iter: Do not assert on MDB_NOTFOUND Balooctl status: Avoid showing 'Content indexing' about folders StatusCommand: Show the correct status for folders SearchStore: Gracefully handle empty term values (bug 356176) Breeze Icons icon updates and additions 22px size status icons for 32px too as you need it in the system tray Changed Fixed to Scalable value to 32px folders in Breeze Dark Extra CMake Modules Make the KAppTemplate CMake module global Silence CMP0063 warnings with KDECompilerSettings ECMQtDeclareLoggingCategory: Include with the generated file Fix CMP0054 warnings KActivities Streamlined the QML loading for KCM (bug 356832) Work-around for the Qt SQL bug that does not clean up connections properly (bug 348194) Merged a plugin that executes applications on activity state change Port from KService to KPluginLoader Port plugins to use kcoreaddons_desktop_to_json() KBookmarks Fully initialize DynMenuInfo in return value KCMUtils KPluginSelector::addPlugins: fix assert if 'config' parameter is default (bug 352471) KCodecs Avoid deliberately overflowing a full buffer KConfig Ensure group is unescaped properly in kconf_update KCoreAddons Add KAboutData::fromPluginMetaData(const KPluginMetaData &plugin) Add KPluginMetaData::copyrightText(), extraInformation() and otherContributors() Add KPluginMetaData::translators() and KAboutPerson::fromJson() Fix use-after-free in desktop file parser Make KPluginMetaData constructible from a json path desktoptojson: make missing service type file an error for the binary make calling kcoreaddons_add_plugin without SOURCES an error KDBusAddons Adapt to Qt 5.6's dbus-in-secondary-thread KDeclarative [DragArea] Add dragActive property [KQuickControlsAddons MimeDatabase] Expose QMimeType comment KDED kded: adapt to Qt 5.6's threaded dbus: messageFilter must trigger module loading in the main thread KDELibs 4 Support kdelibs4support requires kded (for kdedmodule.desktop) Fix CMP0064 warning by setting policy CMP0054 to NEW Don't export symbols that also exist in KWidgetsAddons KDESU Don't leak fd when creating socket KHTML Windows: remove kdewin dependency KI18n Document the first argument rule for plurals in QML Reduce unwanted type changes Make it possible to use doubles as index for i18np*() calls in QML KIO Fix kiod for Qt 5.6's threaded dbus: messageFilter must wait until the module is loaded before returning Change the error code when pasting/moving into a subdirectory Fix emptyTrash blocked issue Fix wrong button in KUrlNavigator for remote URLs KUrlComboBox: fix returning an absolute path from urls() kiod: disable session management Add autocompletion for '.' input which offers all hidden files/folders* (bug 354981) ktelnetservice: fix off by one in argc check, patch by Steven Bromley KNotification [Notify By Popup] Send along event ID Set default non-empty reason for screen saver inhibition; (bug 334525) Add a hint to skip notifications grouping (bug 356653) KNotifyConfig [KNotifyConfigWidget] Allow selecting a specific event Package Framework Make it possible to provide the metadata in json KPeople Fix possible double deletion in DeclarativePersonData KTextEditor Syntax h/l for pli: builtin functions added, expandable regions added KWallet Framework kwalletd: Fix FILE* leak KWindowSystem Add xcb variant for static KStartupInfo::sendFoo methods NetworkManagerQt make it work with older NM versions Plasma Framework [ToolButtonStyle] Always indicate activeFocus Use the SkipGrouping flag for the "widget deleted" notification (bug 356653) Deal properly with symlinks in path to packages Add HiddenStatus for plasmoid self-hiding Stop redirecting windows when item is disabled or hidden. (bug 356938) Don't emit statusChanged if it hasn't changed Fix element ids for east orientation Containment: Don't emit appletCreated with null applet (bug 356428) [Containment Interface] Fix erratic high precision scrolling Read KPluginMetada's property X-Plasma-ComponentTypes as a stringlist [Window Thumbnails] Don't crash if Composite is disabled Let containments override CompactApplet.qml http://kde.org/announcements/kde-frameworks-5.18.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: KDE Frameworks 5.18.0 released
On Sunday 10 January 2016 13:55:58 Andreas Müller wrote: > > Trying to download I get 'forbidden' and in > http://download.kde.org/stable/frameworks/ 5.18 is not displayed. Fixed, thanks for letting me know. I fixed my script so that the chmod only happens on initial creation, won't happen again. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 126249: Fix apidoc format
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126249/#review91465 --- Ship it! Ship It! - David Faure On Jan. 23, 2016, 10:25 a.m., Frederik Schwarzer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126249/ > --- > > (Updated Jan. 23, 2016, 10:25 a.m.) > > > Review request for kdelibs and David Faure. > > > Repository: kio > > > Description > --- > > A full-stop within the first sentence (short description) breaks the line > there. See e.g. > http://api.kde.org/frameworks-api/frameworks5-apidocs/kio/html/namespaceKIO.html#a17631774b47cddb0127d8a3c1fc2315c > > > Diffs > - > > src/core/storedtransferjob.h 3f267c9 > src/core/transferjob.h 6d78793 > > Diff: https://git.reviewboard.kde.org/r/126249/diff/ > > > Testing > --- > > > Thanks, > > Frederik Schwarzer > >
Re: Review Request 120127: Don't show overwrite dialog if file name is empty
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120127/#review91466 --- Ship it! Ship It! - David Faure On Sept. 12, 2014, 8:09 a.m., Ignaz Forster wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120127/ > --- > > (Updated Sept. 12, 2014, 8:09 a.m.) > > > Review request for kdelibs. > > > Repository: kdelibs > > > Description > --- > > Programs using KFileDialog's setConfirmOverwrite(true) (e.g. LibreOffice with > KDE4 integration, Mozilla products with KDE4 integration, several components > of the PIM suite) will trigger the following strange behaviour: > If the file name in the save file dialog is empty and the user selects > "Save", the dialog will ask if you want to overwrite the current directory > (instead of just ignoring the empty input). > > By moving the query below the directory checks the order is correct again, > showing the overwrite confirmation dialog only if the field contains a unique > filename. > > Looking at the code this also affects Frameworks 5 / kio, though I haven't > tested it yet. > > > Diffs > - > > kfile/kfilewidget.cpp fc9b169 > > Diff: https://git.reviewboard.kde.org/r/120127/diff/ > > > Testing > --- > > > Thanks, > > Ignaz Forster > >
KDE Frameworks 5.19.0 released
ut scheme Fix listing of shortcut files (wrong QDir usage) NetworkManagerQt Re-check connection state and other properties to be sure they are actual (version 2) (bug 352326) Oxygen Icons Remove broken linked files Add app icons from the kde applications Add breeze places icons into oxygen Sync oxygen mimetype icons with breeze mimetype icons Plasma Framework Add a property separatorVisible More explicit removal from m_appletInterfaces (bug 358551) Use complementaryColorScheme from KColorScheme AppletQuickItem: Don't try to set initial size bigger than parent size (bug 358200) IconItem: Add usesPlasmaTheme property Don't load toolbox on types not desktop or panel IconItem: Try to load QIcon::fromTheme icons as svg (bug 353358) Ignore check if just one part of size is zero in compactRepresentationCheck (bug 358039) [Units] Return at least 1ms for durations (bug 357532) Add clearActions() to remove every applet interface action [plasmaquick/dialog] Don't use KWindowEffects for Notification window type Deprecate Applet::loadPlasmoid() [PlasmaCore DataModel] Don't reset model when a source is removed Fix margin hints in opague panel background SVG IconItem: Add animated property [Unity] Scale Desktop icon size the button is compose-over-borders paintedWidth/paintedheight for IconItem http://kde.org/announcements/kde-frameworks-5.19.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 127006: konqueror: eliminate signal and shortcut warnings
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127006/#review92312 --- Ship it! (feel free to add me as reviewer for konqueror patches, I only noticed this one by chance) - David Faure On Feb. 7, 2016, 9:07 p.m., Jonathan Marten wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127006/ > --- > > (Updated Feb. 7, 2016, 9:07 p.m.) > > > Review request for KDE Base Apps. > > > Repository: kde-baseapps > > > Description > --- > > These changes to Konqueror's setting up of actions eliminate the runtime > debug messages: > > konqueror(3420)/default err_method_notfound: QObject::connect: No such signal > QAction::triggered(Qt::MouseButtons,Qt::KeyboardModifiers) in > /ws/frameworks/applications/kdebaseapps/konqueror/src/konqmainwindow.cpp:3637 > > and one for each action: > > konqueror(3420)/default KXMLGUIFactoryPrivate::saveDefaultActionProperties: > Shortcut for action "new_window" "New &Window" set with > QAction::setShortcut()! Use KActionCollection::setDefaultShortcut(s) instead. > > and also a number of compiler warnings: > > konqueror/src/konqmainwindow.cpp:3705: warning: ‘KShortcut’ is deprecated > > > Diffs > - > > konqueror/src/konqmainwindow.cpp c7a81c8 > > Diff: https://git.reviewboard.kde.org/r/127006/diff/ > > > Testing > --- > > Built Konqueror with these changes. Observed absence of those compiler and > runtime error messages, checked correct operation of key shortcuts. > > > Thanks, > > Jonathan Marten > >
Re: Review Request 127138: Konqueror: Fix window title showing as "/" when URL ends with that
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127138/#review92652
---
konqueror/src/konqview.cpp (line 692)
<https://git.reviewboard.kde.org/r/127138/#comment63155>
Your code is correct in that it's equivalent to the old code. However I
wonder if this couldn't be simplified.
If we want to display the filename of this->url(), unless "caption" was set
to something completely different by the part, we could just compare path()
instead of fileName()...
const QUrl captionUrl(QUrl::fromUserInput(caption));
if (captionUrl.isValid() && captionUrl.isLocalFile() &&
captionUrl.path() == url().path()) {
adjustedCaption =
captionUrl.adjusted(QUrl::StripTrailingSlash).fileName();
// if empty...
}
This looks a bit simpler, I think? If you agree, can you test it, and push
it if it works?
- David Faure
On Feb. 22, 2016, 3:12 p.m., Jonathan Marten wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127138/
> ---
>
> (Updated Feb. 22, 2016, 3:12 p.m.)
>
>
> Review request for KDE Base Apps and David Faure.
>
>
> Repository: kde-baseapps
>
>
> Description
> ---
>
> This happens when a local file URL which ends in "/" is navigated to. For
> example, going to "/home/user" correctly shows "user" as the window title,
> while going to "/home/user/" shows "/" because the fileName() of that URL is
> empty. This happens in particular when using the "Up" action, as it goes
> from "/home/user/foo" to "/home/user/".
>
> This change ensures that trailing slashes are removed from URLs before using
> fileName() on them.
>
>
> Diffs
> -
>
> konqueror/src/konqview.cpp 3707c7a
>
> Diff: https://git.reviewboard.kde.org/r/127138/diff/
>
>
> Testing
> ---
>
> Built kde-baseapps with these changes, observed correct display of base name
> in Konqueror window title while navigating.
>
>
> Thanks,
>
> Jonathan Marten
>
>
Re: [Kde-pim] PIM dependency metadata
On Sunday 28 February 2016 08:09:38 Ben Cooksley wrote: > On Sun, Feb 28, 2016 at 1:51 AM, Sandro Knauß wrote: > > Hey, > > > >> There are a couple of issues, namely: > >> a) For some reason it depends on plasma-workspace - this seems > >> completely wrong to me. Please be careful when adding items to the > >> dependency chain > > > > this was added because of ktimezoned, that is part of plasma-workspace to > > run > > tests in kcalccore successfully. For further discussion see: > > https://phabricator.kde.org/T1147 > > ktimezoned needs to move out of Plasma then, or the relevant classes > need to be rewritten Yep, this sounds like ktimezoned should move to KF5. How are non-plasma users supposed to get correct timezones in e.g. PIM apps otherwise? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: mailing list archive gone ?
On Sunday 28 February 2016 15:42:29 Martin Koller wrote: > > it says: https://mail.kde.org/mailman/listinfo/kde-core-devel I have now updated this page to point to the archives (thanks to Ingo Klöcker for this help). -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Review Request 127264: Set CMP0064 to OLD to suppress CMake warnings
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127264/#review93401 --- I don't really see how it's the same idea as a commit where I set CMP0054 (not 64) to NEW (not OLD). Setting a policy to OLD means "I know I'm using something deprecated and I don't care", which isn't exactly a good solution in the long term. Can you try actually doing the same as that other commit, i.e. setting CMP0054 to NEW instead? - David Faure On March 3, 2016, 12:58 p.m., Rohan Garg wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127264/ > --- > > (Updated March 3, 2016, 12:58 p.m.) > > > Review request for kdelibs and David Faure. > > > Repository: kdelibs > > > Description > --- > > This follows the same idea from 826a5ff3278f492a99ac6827614e1d0ca40a45e8 > > > Diffs > - > > cmake/modules/KDE4Macros.cmake 5bb2ffa > > Diff: https://git.reviewboard.kde.org/r/127264/diff/ > > > Testing > --- > > > Thanks, > > Rohan Garg > >
Re: Product versions on bugs.kde.org
On Tuesday 08 March 2016 23:09:33 Kevin Funk wrote: > On Monday, March 7, 2016 1:23:01 PM CET Kevin Funk wrote: > > Heh, nice one! > > > > I'll try this out on Frameworks when I find the time. > > Added all versions from 5.5.0 to 5.19.0. Wow. Kevin and Jonathan, you're my heroes ! > http://paste.ubuntu.com/15330672/ > > @dfaure: Do you want me to commit that somewhere (obviously with just a > single > "version" instead of many), so you can simply invoke that script when > releasing a new KF5 version? Yes, please add it to g...@git.kde.org:sysadmin/release-tools branch frameworks/5.0 Ideally as part of create_sources_inc so I don't have to run yet another script ;-) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 signature.asc Description: This is a digitally signed message part.
KDE Frameworks 5.20.0 released
sure KDocTools is looked-up Don't pass a negative number to dbus, it asserts in libdbus Clean cmake files KWallet::openWallet(Synchronous): don't time out after 25 seconds KWindowSystem support _NET_WM_BYPASS_COMPOSITOR (bug 349910) KXMLGUI Use non-native Language name as fallback Fix session management broken since KF5 / Qt5 (bug 354724) Shortcut schemes: support globally installed schemes Use Qt's qHash(QKeySequence) when building against Qt 5.6+ Shortcut schemes: fix bug where two KXMLGUIClients with the same name overwrite each other's scheme file kxmlguiwindowtest: add shortcuts dialog, for testing the shortcut schemes editor Shortcut schemes: improve usability by changing texts in GUI Shortcut schemes: improve scheme list combo (automatic size, don't clear on unknown scheme) Shortcut schemes: don't prepend the guiclient name to the filename Shortcut schemes: create dir before trying to save a new shortcut scheme Shortcut schemes: restore layout margin, it looks very cramped otherwise Fix memory leak in KXmlGui startup hook Plasma Framework IconItem: Don't overwrite source when using QIcon::name() ContainmentInterface: Fix use of QRect right() and bottom() Remove effectively duplicate code path for handling QPixmaps Add API docs for IconItem Fix stylesheet (bug 359345) Don't wipe window mask on every geometry change when compositing is active and no mask has been set Applet: Don't crash on remove panel (bug 345723) Theme: Discard pixmap cache when changing theme (bug 359924) IconItemTest: Skip when grabToImage fails IconItem: Fix changing color of svg icons loaded from icon theme Fix svg iconPath resolving in IconItem If path is passed, pick the tail (bug 359902) Add properties configurationRequired and reason Move contextualActionsAboutToShow to Applet ScrollViewStyle: Do not use margins of the flickable item DataContainer: Fix slot checks before connect/disconnect ToolTip: Prevent multiple geometry changes while changing contents SvgItem: Don't use Plasma::Theme from rendering thread AppletQuickItem: Fix finding own attached layout (bug 358849) Smaller expander for the taskbar ToolTip: Stop show timer if hideTooltip is called (bug 358894) Disable animation of icons in plasma tooltips Drop animations from tooltips Default theme follows color scheme Fix IconItem not loading non-theme icons with name (bug 359388) Prefer other containments than desktop in containmentAt() WindowThumbnail: Discard glx pixmap in stopRedirecting() (bug 357895) Remove the legacy applets filter ToolButtonStyle: Don't rely on an outside ID Don't assume we find a corona (bug 359026) Calendar: Add proper back/forward buttons and a "Today" button (bugs 336124, 348362, 358536) Sonnet Don't disable language detection just because a language is set Disable automatic disabling of automatic spelling by default Fix TextBreaks Fix Hunspell dictionary search paths missing '/' (bug 359866) Add /../share/hunspell to dictionary search path http://kde.org/announcements/kde-frameworks-5.20.0.php -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Product versions on bugs.kde.org
On Saturday 12 March 2016 20:38:43 Vishesh Handa wrote: > > And from my point of view the user doesn't care if Baloo is part of > something called "Frameworks". There is a bug, they want to report it. > It's just added noise. Typically, users report bugs in the app where they see them, and then the maintainer of the app reassigns to the "guilty" underlying framework. E.g. end users don't report kio bugs, they report dolphin bugs, which then get reassigned to frameworks-kio. So I think frameworks-baloo makes sense (and is consistent). The "users" of the framework are application developers, who know how to find it in bugzilla. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Product versions on bugs.kde.org
On Wednesday 16 March 2016 21:40:22 Vishesh Handa wrote: > On Sun, Mar 13, 2016 at 7:57 PM, David Faure wrote: > > On Saturday 12 March 2016 20:38:43 Vishesh Handa wrote: > >> > >> And from my point of view the user doesn't care if Baloo is part of > >> something called "Frameworks". There is a bug, they want to report it. > >> It's just added noise. > > > > Typically, users report bugs in the app where they see them, and then > > the maintainer of the app reassigns to the "guilty" underlying framework. > > > > E.g. end users don't report kio bugs, they report dolphin bugs, which then > > get reassigned to frameworks-kio. > > > > So I think frameworks-baloo makes sense (and is consistent). The "users" > > of the framework are application developers, who know how to find it in > > bugzilla. > > Except that Baloo isn't just a library. It provides a daemon, and a > CLI search interface. I also know a few people running Baloo on > servers. > > This whole "frameworks-baloo" vs "baloo" seems more pedantic than > anything else. Specially considering the amount of overhead this is > actively causing. I just have a crash report from 2 hours ago filed to > Baloo (not Baloo-frameworks). Bug reports against previous versions of > Baloo won't automatically go to this new "baloo-frameworks". And I > have over 130+ new bug emails, which aren't relevant. > > I'm all for consistency, but not when it has a real cost. I'm stepping out of this discussion, do whatever you want :-) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
Re: Product versions on bugs.kde.org
On Sunday 20 March 2016 18:43:04 Alexander Potashev wrote: > 2016-03-09 1:09 GMT+03:00 Kevin Funk : > > Added all versions from 5.5.0 to 5.19.0. > > 5.20.0 is out, could you please add it to Bugzilla as well? I have now integrated Jonathan's script into release-tools, and I ran it for 5.20.0. Works well. There's just one issue: the bugzilla product for attica is called attica and not frameworks-attica, unlike all other frameworks. Should we rename the product in bugzilla, or adjust the script? PS: we still have no official attica maintainer in metainfo.yaml, but the bugzilla component is assigned to Frederik Gladhorn. Frederik, can I write you down in metainfo.yaml as well? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5
