Re: What's qt5-stable on build.kde.org
On 18 Sep 2014, at 07:52 , Martin Gräßlin mgraess...@kde.org wrote: That's not what I meant. I mean we have things like frameworks which require only Qt 5.2, but we also have things like e.g. KWin requiring Qt 5.3 (soon 5.4). This will require two Qt builds, while we currently have only one. But it doesn't require that everything is compiled twice. ah, okay. So, how do you organise access to that alternative qt5 version then? Would you bring in stg like a qt5_2nd-master_qt5 in order to achieve that consistently on the CI system itself? Greets, Marko signature.asc Description: Message signed with OpenPGP using GPGMail ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 120257: Add support for initial mapping state of WM_HINTS
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120257/ --- Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- To complement the other flags already taken from WM_HINTS, we also read the initial mapping state. Diffs - autotests/netwininfotestclient.cpp 2795cbf11277d6dc78d24979797e6d6c9cd3750e src/netwm.h 0535c39efc3d4fc428545abd599ce927fbf8903d src/netwm.cpp 3107a2388c2fb3f1a8ad97e466be40e6977e064b src/netwm_def.h c8b1ba0a99a5f1e8a4939d0304b0facde25c59a8 src/netwm_p.h 3bb8213acd39f86c8c1c49015243d2bd3e82a8ba Diff: https://git.reviewboard.kde.org/r/120257/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: What's qt5-stable on build.kde.org
On Thu, Sep 18, 2014 at 6:04 PM, Marko Käning mk-li...@email.de wrote: On 18 Sep 2014, at 07:52 , Martin Gräßlin mgraess...@kde.org wrote: That's not what I meant. I mean we have things like frameworks which require only Qt 5.2, but we also have things like e.g. KWin requiring Qt 5.3 (soon 5.4). This will require two Qt builds, while we currently have only one. But it doesn't require that everything is compiled twice. ah, okay. So, how do you organise access to that alternative qt5 version then? Would you bring in stg like a qt5_2nd-master_qt5 in order to achieve that consistently on the CI system itself? Unfortunately Martin, due to the way the CI system is designed we would have to compile everything twice. The setup is explicitly designed to prohibit any form of version mixing at all. This would still apply even after the implementation of new scheme for dependencies and other build metadata. Greets, Marko Regards, Ben ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: What's qt5-stable on build.kde.org
On Thursday, 18 September 2014 10:10:41 CEST, Ben Cooksley wrote: This would still apply even after the implementation of new scheme for dependencies and other build metadata. Where do I learn about these new features? Cheers, Jan -- Trojitá, a fast Qt IMAP e-mail client -- http://trojita.flaska.net/ ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: What's qt5-stable on build.kde.org
On Thu, Sep 18, 2014 at 9:30 PM, Jan Kundrát j...@flaska.net wrote: On Thursday, 18 September 2014 10:10:41 CEST, Ben Cooksley wrote: This would still apply even after the implementation of new scheme for dependencies and other build metadata. Where do I learn about these new features? See the mail thread with the subject Proposal to improving KDE Software Repository Organization in the kde-core-devel or kde-frameworks-devel mailing list archives. Cheers, Jan Thanks, Ben -- Trojitá, a fast Qt IMAP e-mail client -- http://trojita.flaska.net/ ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119607: Support for .hidden files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/ --- (Updated Set. 18, 2014, 10:06 a.m.) Review request for KDE Frameworks and David Faure. Changes --- Implemented some of the suggestions. Haven't tested yet. Bugs: 64740 and 246260 https://bugs.kde.org/show_bug.cgi?id=64740 https://bugs.kde.org/show_bug.cgi?id=246260 Repository: kio Description --- This adds support for *.hidden* files to KDE. When listing a directory, the files/folders listed in the *.hidden* file will be hidden, unless the user has chosen to show hidden files. This patch was initially based on the patch provided by Mark in Bug #246260. Diffs (updated) - src/core/kcoredirlister.h e6ba2ac src/core/kcoredirlister.cpp a31d629 src/core/kfileitem.h bebc241 src/core/kfileitem.cpp 74dc069 Diff: https://git.reviewboard.kde.org/r/119607/diff/ Testing --- Built and tested the patch in Project Neon. Dolphin was still using KDE4/Qt4 version of the library, so I only tested using the desktop folder widget, and folder desktop. It worked correctly when I pointed it to ~ and ~/Desktop (I added .hidden there). However, it didn't work when I pointed it to the Desktop folder (the default option, not the custom location ~/Desktop). More testing is required. The version for KDE4/Qt4 submitted to Bug #246260 was tested in Kubuntu 14.04, and it worked everywhere I tested (Dolphin, open/save dialogs, folder widget and detailed/tree view in Dolphin). It wasn't an intensive test, though. Thanks, Bruno Nova ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119607: Support for .hidden files
On Set. 17, 2014, 7:59 p.m., David Faure wrote: Thanks for the patch. A few comments: Is there any sort of spec or reference implementation for .hidden files? Are they supposed to support wildcards, for instance? Something else, it would be good to add a unittest to autotests/kdirlistertest.cpp (or autotests/kdirmodeltest.cpp). OK, I'll try to add a unittest later. I've searched for a general specification for .hidden files, but have found none. However, I have accidentally found, through Wikipedia, the commit that adds .hidden support to GNOME glib: https://git.gnome.org/browse/glib/commit/?id=510ba9b4efe1813e24c6dfa7405c3547bf9efdd7 On Set. 17, 2014, 7:59 p.m., David Faure wrote: src/core/kfileitem.h, line 260 https://git.reviewboard.kde.org/r/119607/diff/3/?file=312279#file312279line260 @since 5.3 OK. Added it to the comment below that line. On Set. 17, 2014, 7:59 p.m., David Faure wrote: src/core/kcoredirlister.cpp, line 2811 https://git.reviewboard.kde.org/r/119607/diff/3/?file=312278#file312278line2811 not needed, the QFile destructor does it Didn't know that, thanks! Line removed. On Set. 17, 2014, 7:59 p.m., David Faure wrote: src/core/kcoredirlister.cpp, line 2801 https://git.reviewboard.kde.org/r/119607/diff/3/?file=312278#file312278line2801 Declare where used first, no need to put it here. `name` is used first inside of a loop, that's why I declared it outside. That gives me the impression that it's only created and destroyed once. Then again, maybe there's no impact and the compiler optimizes that. I moved the declaration to where it's first used. - Bruno --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/#review66777 --- On Set. 18, 2014, 10:06 a.m., Bruno Nova wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119607/ --- (Updated Set. 18, 2014, 10:06 a.m.) Review request for KDE Frameworks and David Faure. Bugs: 64740 and 246260 https://bugs.kde.org/show_bug.cgi?id=64740 https://bugs.kde.org/show_bug.cgi?id=246260 Repository: kio Description --- This adds support for *.hidden* files to KDE. When listing a directory, the files/folders listed in the *.hidden* file will be hidden, unless the user has chosen to show hidden files. This patch was initially based on the patch provided by Mark in Bug #246260. Diffs - src/core/kcoredirlister.h e6ba2ac src/core/kcoredirlister.cpp a31d629 src/core/kfileitem.h bebc241 src/core/kfileitem.cpp 74dc069 Diff: https://git.reviewboard.kde.org/r/119607/diff/ Testing --- Built and tested the patch in Project Neon. Dolphin was still using KDE4/Qt4 version of the library, so I only tested using the desktop folder widget, and folder desktop. It worked correctly when I pointed it to ~ and ~/Desktop (I added .hidden there). However, it didn't work when I pointed it to the Desktop folder (the default option, not the custom location ~/Desktop). More testing is required. The version for KDE4/Qt4 submitted to Bug #246260 was tested in Kubuntu 14.04, and it worked everywhere I tested (Dolphin, open/save dialogs, folder widget and detailed/tree view in Dolphin). It wasn't an intensive test, though. Thanks, Bruno Nova ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120160: KMessageWidget: fix handling of rapid succession of animatedHide+animatedShow calls
On Sept. 13, 2014, 3:20 p.m., Milian Wolff wrote: autotests/kmessagewidgetautotest.cpp, line 115 https://git.reviewboard.kde.org/r/120160/diff/1/?file=311761#file311761line115 Couldn't this actually check that no animation is running at all now? We triggered the hide directly after the show, without going into the eventloop inbetween. I'd expect no animation to be triggered at all. generally, I'd urge you to split this test function into smaller ones and give each test a good name so that its clear what you check. Then also add a case for the case you try to test here, but don't actually (as far as I can see). Namely: w.animatedShow(); QTest::qWait(10); QVERIFY(w.isShowAnimationRunning()); w.animatedHide(); QVERIFY(w.isHideAnimationRunning()); QTest::qWait(10); QVERIFY(!w.isHideAnimationRunning()); QVERIFY(!w.isVisible()); and similar for the animatedShow/Hide when it is shown already. Couldn't this actually check that no animation is running at all now? - Well, with the current implementation that would not be true because, as far as I can see, the QTimeLine stays in running state (see updated test in patch v2). I don't know QTimeLine internals, but I guess it needs to process some events before it can notice that there's actually nothing to do. In the new version of the patch, I've simplified the `while (w.isShowAnimationRunning() || w.isHideAnimationRunning())` so that it only checks one condition instead of two. Previously I had put these two checks because unpatched KMessageWidget runs the wrong animation and I wanted to wait for *any* animation to finish before checking the outcome, but in the new version I've added a `QVERIFY(w.isHideAnimationRunning())` before the loop, which guarantees that the correct animation is running. About the splitting, I'm using the widget height to test if the message is fully shown or hidden (see `shownHeight` and `hiddenHeight` variables). These values are first obtained through simple single `animatedShow`/`animatedHide` calls in the first part of the test, and then used in the second part of the test as reference height. So, if I split the test into smaller pieces, I would not have this data anymore and I'd have to drop the height checks. So we have two options: one big test that checks the height, several small tests that don't check the height. The height check is not strictly necessary, I've added it for extra safety, what do you think? About your last comment, actually I am testing if `animatedShow` immediatly followed by `animatedHide` works (and vice versa). But I think I see your point, so I've added a `QVERIFY(w.isShowAnimationRunning())` after every `w.animatedShow()` and a `QVERIFY(w.isHideAnimationRunning())` after every `w.animatedHide()`, what do you think, is it better now? On the other hand there's no need to insert qWait between `animatedShow` and `animatedHide`, because it would still test the same code paths, at least given the current implementation. Also, I'm not a huge fan of the fixed timining you're proposing, because it would depend on the first `QTest::qWait(10)` to sleep no longer than the second one, which is something the OS can't actually guarantee. - Fabio --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/#review66411 --- On Sept. 18, 2014, 10:40 a.m., Fabio D'Urso wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/ --- (Updated Sept. 18, 2014, 10:40 a.m.) Review request for KDE Frameworks, Christoph Feck and Aurélien Gâteau. Repository: kwidgetsaddons Description --- Hi, I've found that if you call animatedShow() immediatly after animatedHide(), without waiting for the hide animation to finish first, the hide animation goes on and, in the end, the message widget becomes hidden. The attached patch adds checks in animatedShow and animatedHide so that, if an opposite animation is currently running, they just reverse it. Diffs - autotests/kmessagewidgetautotest.h 062e2b3 autotests/kmessagewidgetautotest.cpp f46bab0 src/kmessagewidget.cpp bc7ba32 tests/kmessagewidgettest.cpp f621b5a Diff: https://git.reviewboard.kde.org/r/120160/diff/ Testing --- I've added buttons to invoke animatedShow and animatedHide in tests/kmessagewidget.cpp and used them to test the behavior. I've also added an autotest that checks the final height and isVisible() value, both after single animatedShow/animatedHide calls and after
Re: Review Request 120160: KMessageWidget: fix handling of rapid succession of animatedHide+animatedShow calls
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/ --- (Updated Sept. 18, 2014, 10:40 a.m.) Review request for KDE Frameworks, Christoph Feck and Aurélien Gâteau. Changes --- Added some QVERIFY(w.isShowAnimationRunning()) and QVERIFY(w.isHideAnimationRunning()) to the test Repository: kwidgetsaddons Description --- Hi, I've found that if you call animatedShow() immediatly after animatedHide(), without waiting for the hide animation to finish first, the hide animation goes on and, in the end, the message widget becomes hidden. The attached patch adds checks in animatedShow and animatedHide so that, if an opposite animation is currently running, they just reverse it. Diffs (updated) - autotests/kmessagewidgetautotest.h 062e2b3 autotests/kmessagewidgetautotest.cpp f46bab0 src/kmessagewidget.cpp bc7ba32 tests/kmessagewidgettest.cpp f621b5a Diff: https://git.reviewboard.kde.org/r/120160/diff/ Testing --- I've added buttons to invoke animatedShow and animatedHide in tests/kmessagewidget.cpp and used them to test the behavior. I've also added an autotest that checks the final height and isVisible() value, both after single animatedShow/animatedHide calls and after animatedShow+animatedHide and animatedHide+animatedShow. Thanks, Fabio D'Urso ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120160: KMessageWidget: fix handling of rapid succession of animatedHide+animatedShow calls
On Sept. 14, 2014, 6:33 p.m., Dominik Haumann wrote:
All in all, the patch looks good, but it misses emitting the signals
hideAnimationFinished() and showAnimationFinished().
Why? Without your patch, you can be *sure* that after calling show() or
animatedShow() the signal showAnimationFinished() will eventually be
emitted.
With the current version of your patch, this is not true anymore. For
instance, KatePart relies on these signals to work.
I'd be fine with committing this to KF5, but I would suggest to not
backport to 4.x.
Well, iirc I've read that KatePart internally queues messages and waits for the
completion signal before showing the next message, therefore it never
interrupts running animations (which is what I'm fixing), or does it?
About the 4.x backport, we're agreed: I'm targeting kf5 only (4.x doesn't even
have `is{Hide,Show}AnimationRunning` and `{hide,show}AnimationFinished`)
On Sept. 14, 2014, 6:33 p.m., Dominik Haumann wrote:
src/kmessagewidget.cpp, line 411
https://git.reviewboard.kde.org/r/120160/diff/1/?file=311762#file311762line411
I believe this is missing after line 411:
emit showAnimationFinished();
Sprry, I'm not following you, do you mean the other way round? If you call
`animatedShow` and `isHideAnimationRunning()` is currently true there's no way
I'd expect a `showAnimationFinished()` _now_. I *might* expect a
`hideAnimationFinished()`, but actually not, because the hide animation was
just interrupted, not really finished.
On Sept. 14, 2014, 6:33 p.m., Dominik Haumann wrote:
src/kmessagewidget.cpp, line 442
https://git.reviewboard.kde.org/r/120160/diff/1/?file=311762#file311762line442
I believe this is missing after line 441:
emit hideAnimationFinished();
Same as above, did you mean `showAnimationFinished`?
- Fabio
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120160/#review66489
---
On Sept. 18, 2014, 10:40 a.m., Fabio D'Urso wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120160/
---
(Updated Sept. 18, 2014, 10:40 a.m.)
Review request for KDE Frameworks, Christoph Feck and Aurélien Gâteau.
Repository: kwidgetsaddons
Description
---
Hi, I've found that if you call animatedShow() immediatly after
animatedHide(), without waiting for the hide animation to finish first, the
hide animation goes on and, in the end, the message widget becomes hidden.
The attached patch adds checks in animatedShow and animatedHide so that, if
an opposite animation is currently running, they just reverse it.
Diffs
-
autotests/kmessagewidgetautotest.h 062e2b3
autotests/kmessagewidgetautotest.cpp f46bab0
src/kmessagewidget.cpp bc7ba32
tests/kmessagewidgettest.cpp f621b5a
Diff: https://git.reviewboard.kde.org/r/120160/diff/
Testing
---
I've added buttons to invoke animatedShow and animatedHide in
tests/kmessagewidget.cpp and used them to test the behavior.
I've also added an autotest that checks the final height and isVisible()
value, both after single animatedShow/animatedHide calls and after
animatedShow+animatedHide and animatedHide+animatedShow.
Thanks,
Fabio D'Urso
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119607: Support for .hidden files
On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote:
src/core/kcoredirlister.cpp, line 1218
https://git.reviewboard.kde.org/r/119607/diff/2/?file=301215#file301215line1218
const
Bruno Nova wrote:
What do you mean? `filesToHide` should be `const`?
(I thought I had added this comment before, but I'm not seeing it)
David Faure wrote:
Yes, make local vars const if they are not getting modified. Helps
readability and helps avoiding unnecessary detach()s, e.g. when calling
begin().
Never thought about readability. Good point.
Added the consts.
On Set. 14, 2014, 3:27 p.m., Frank Reininghaus wrote:
src/core/kcoredirlister.cpp, line 2799
https://git.reviewboard.kde.org/r/119607/diff/2/?file=301215#file301215line2799
This will only work for local files. I'm not sure if we would want to
support the .hidden mechanism also for other protocols, such as sftp or
fish?
Moreover, I think that we might want to cache the contents of the
.hidden file, to prevent that we read it, parse it and construct the QSet
with the hidden files over and over again if a kioslave reports the
UDSEntries in multiple batches.
David Faure wrote:
Supporting .hidden for remote protocols would be much more complex (async
API).
Caching: it would have to include an mtime, to be able to detect changes.
But indeed caching the last one read would be good, because while having a
directory open, any change triggers an update job, which will re-read the
.hidden file. Or while at it, could be a LRU cache, Qt makes it simple:
struct CacheHiddenFile {
QDateTime mtime;
QSetQString listedFiles;
}
QCacheQString /*dot hidden file*/, CacheHiddenFile
m_cacheHiddenFiles;
m_cacheHiddenFiles.setMaxCost(10);
This requires making the filesInDotHiddenForDir method not file-static as
I previously suggested, but rather a method of KCoreDirLister::Private, which
would also have the above member.
Now that you mention remote protocols, I just found out that Nautilus .hidden
also doesn't support them.
For example, .hidden is not interpreted when I access a computer through SFTP
in Nautilus. However, when I mount it externally using `sshfs`, it works.
Haven't checked Thunar.
As for the caching, I'll try to take a look at it, but I'll probably need help
(I have very little experience with KDE and Qt).
- Bruno
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119607/#review66475
---
On Set. 18, 2014, 10:06 a.m., Bruno Nova wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119607/
---
(Updated Set. 18, 2014, 10:06 a.m.)
Review request for KDE Frameworks and David Faure.
Bugs: 64740 and 246260
https://bugs.kde.org/show_bug.cgi?id=64740
https://bugs.kde.org/show_bug.cgi?id=246260
Repository: kio
Description
---
This adds support for *.hidden* files to KDE.
When listing a directory, the files/folders listed in the *.hidden* file will
be hidden, unless the user has chosen to show hidden files.
This patch was initially based on the patch provided by Mark in Bug #246260.
Diffs
-
src/core/kcoredirlister.h e6ba2ac
src/core/kcoredirlister.cpp a31d629
src/core/kfileitem.h bebc241
src/core/kfileitem.cpp 74dc069
Diff: https://git.reviewboard.kde.org/r/119607/diff/
Testing
---
Built and tested the patch in Project Neon.
Dolphin was still using KDE4/Qt4 version of the library, so I only tested
using the desktop folder widget, and folder desktop.
It worked correctly when I pointed it to ~ and ~/Desktop (I added
.hidden there).
However, it didn't work when I pointed it to the Desktop folder (the
default option, not the custom location ~/Desktop).
More testing is required.
The version for KDE4/Qt4 submitted to Bug #246260 was tested in Kubuntu
14.04, and it worked everywhere I tested (Dolphin, open/save dialogs, folder
widget and detailed/tree view in Dolphin).
It wasn't an intensive test, though.
Thanks,
Bruno Nova
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Non-coinstallable frameworks
Hi Ivan, I've just learned that the KActivities framework is not co-installable with kdelibs4. We should fix that in a way or another, this makes the whole plasma-framework (or anything else depending on KActivities) not co-installable as well, which is quite bad as some applications still need it in order to reach the components (such as Muon). Is there any way we could fix this? To me it's looking quite bad, maybe it would make sense to move the KActivities dependency from plasma-framework to plasma-workspace? It's barely used in plasma-framework anyway. Aleix ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120160: KMessageWidget: fix handling of rapid succession of animatedHide+animatedShow calls
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/#review66825 --- Ship it! some more nitpickery from my side, but otherwise this is good already. I'd like to see this in and not hold you up much longer. autotests/kmessagewidgetautotest.cpp https://git.reviewboard.kde.org/r/120160/#comment46606 btw, are there signals for these animations? then you could use the much faster approach to wait for the signal via a QSignalSpy. autotests/kmessagewidgetautotest.cpp https://git.reviewboard.kde.org/r/120160/#comment46605 instead, I'd add a QVERIFY(shownHeight) above when you get the value of shownHeight. autotests/kmessagewidgetautotest.cpp https://git.reviewboard.kde.org/r/120160/#comment46607 imo, this should be replaced by an explicit call to the event loop instead of waiting in a loop. If I understand you correctly, the animation just needs to work through the pending events to know its not running anymore. there should not be any time delay involved otherwise. qApp-processEvents(); autotests/kmessagewidgetautotest.cpp https://git.reviewboard.kde.org/r/120160/#comment46608 considering that the show was instantly stopped by a hide, I would say these checks should be done before handling any events and then afterwards as well. autotests/kmessagewidgetautotest.cpp https://git.reviewboard.kde.org/r/120160/#comment46609 similar to above - Milian Wolff On Sept. 18, 2014, 10:40 a.m., Fabio D'Urso wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/ --- (Updated Sept. 18, 2014, 10:40 a.m.) Review request for KDE Frameworks, Christoph Feck and Aurélien Gâteau. Repository: kwidgetsaddons Description --- Hi, I've found that if you call animatedShow() immediatly after animatedHide(), without waiting for the hide animation to finish first, the hide animation goes on and, in the end, the message widget becomes hidden. The attached patch adds checks in animatedShow and animatedHide so that, if an opposite animation is currently running, they just reverse it. Diffs - autotests/kmessagewidgetautotest.h 062e2b3 autotests/kmessagewidgetautotest.cpp f46bab0 src/kmessagewidget.cpp bc7ba32 tests/kmessagewidgettest.cpp f621b5a Diff: https://git.reviewboard.kde.org/r/120160/diff/ Testing --- I've added buttons to invoke animatedShow and animatedHide in tests/kmessagewidget.cpp and used them to test the behavior. I've also added an autotest that checks the final height and isVisible() value, both after single animatedShow/animatedHide calls and after animatedShow+animatedHide and animatedHide+animatedShow. Thanks, Fabio D'Urso ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Minimum compiler versions
2014-09-17 14:12 GMT-03:00 Nicolás Alvarez nicolas.alva...@gmail.com: If we'll keep the compiler requirements as-is, I volunteer to fix what's broken in the minimum versions we claim to support. But I think we need CI for gcc 4.5, otherwise it will keep breaking... Okay, that wasn't so hard. KF5 now compiles with gcc 4.5. Something might have broken on MSVC2010 though, I'll have to try again once I reboot to Windows :) -- Nicolás ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build became unstable: kwindowsystem_master_qt5 » All,LINBUILDER #104
See http://build.kde.org/job/kwindowsystem_master_qt5/Variation=All,label=LINBUILDER/104/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120160: KMessageWidget: fix handling of rapid succession of animatedHide+animatedShow calls
On Sept. 14, 2014, 6:33 p.m., Dominik Haumann wrote:
All in all, the patch looks good, but it misses emitting the signals
hideAnimationFinished() and showAnimationFinished().
Why? Without your patch, you can be *sure* that after calling show() or
animatedShow() the signal showAnimationFinished() will eventually be
emitted.
With the current version of your patch, this is not true anymore. For
instance, KatePart relies on these signals to work.
I'd be fine with committing this to KF5, but I would suggest to not
backport to 4.x.
Fabio D'Urso wrote:
Well, iirc I've read that KatePart internally queues messages and waits
for the completion signal before showing the next message, therefore it never
interrupts running animations (which is what I'm fixing), or does it?
About the 4.x backport, we're agreed: I'm targeting kf5 only (4.x doesn't
even have `is{Hide,Show}AnimationRunning` and `{hide,show}AnimationFinished`)
Yes, I swapped both signals. Well ok, if you do not add these two lines, then
please document that in the header file for the two signals
hideAnimationFinished(), along the lines:
@note Calling animatedShow() while the hide animation is running (see
hideAnimationRunning()) will @e not emit this signals.
Same for showAnimationFinished().
I'd prefer to have a 3rd opinion about this from other developers what they'd
expect.
- Dominik
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120160/#review66489
---
On Sept. 18, 2014, 10:40 a.m., Fabio D'Urso wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120160/
---
(Updated Sept. 18, 2014, 10:40 a.m.)
Review request for KDE Frameworks, Christoph Feck and Aurélien Gâteau.
Repository: kwidgetsaddons
Description
---
Hi, I've found that if you call animatedShow() immediatly after
animatedHide(), without waiting for the hide animation to finish first, the
hide animation goes on and, in the end, the message widget becomes hidden.
The attached patch adds checks in animatedShow and animatedHide so that, if
an opposite animation is currently running, they just reverse it.
Diffs
-
autotests/kmessagewidgetautotest.h 062e2b3
autotests/kmessagewidgetautotest.cpp f46bab0
src/kmessagewidget.cpp bc7ba32
tests/kmessagewidgettest.cpp f621b5a
Diff: https://git.reviewboard.kde.org/r/120160/diff/
Testing
---
I've added buttons to invoke animatedShow and animatedHide in
tests/kmessagewidget.cpp and used them to test the behavior.
I've also added an autotest that checks the final height and isVisible()
value, both after single animatedShow/animatedHide calls and after
animatedShow+animatedHide and animatedHide+animatedShow.
Thanks,
Fabio D'Urso
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120257: Add support for initial mapping state of WM_HINTS
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120257/#review66844 --- Ship it! src/netwm_def.h https://git.reviewboard.kde.org/r/120257/#comment46621 soon we're gonna need NET::Properties3 ;-) - Thomas Lübking On Sept. 18, 2014, 8 vorm., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120257/ --- (Updated Sept. 18, 2014, 8 vorm.) Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- To complement the other flags already taken from WM_HINTS, we also read the initial mapping state. Diffs - autotests/netwininfotestclient.cpp 2795cbf11277d6dc78d24979797e6d6c9cd3750e src/netwm.h 0535c39efc3d4fc428545abd599ce927fbf8903d src/netwm.cpp 3107a2388c2fb3f1a8ad97e466be40e6977e064b src/netwm_def.h c8b1ba0a99a5f1e8a4939d0304b0facde25c59a8 src/netwm_p.h 3bb8213acd39f86c8c1c49015243d2bd3e82a8ba Diff: https://git.reviewboard.kde.org/r/120257/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 120267: Don't run kioexec if we are already running it
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120267/ --- Review request for KDE Frameworks and David Faure. Bugs: 339123 https://bugs.kde.org/show_bug.cgi?id=339123 Repository: kio Description --- Only add the actual command and url's otherwise we keep spawning instances of kioexec. Maybe we should rename the kioexec service from 'dummy' to 'kioexec-dummy' just to be safe. Diffs - src/core/desktopexecparser.cpp 9510697 Diff: https://git.reviewboard.kde.org/r/120267/diff/ Testing --- Run kioexec with local file from command line. Remote files don't work (before and after) I'll take a look at that next. Thanks, Maarten De Meyer ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120155: Correctly load non-square icons in KIconLoader
On Sept. 12, 2014, 10:49 a.m., Christoph Feck wrote: Use KPixmapSequence if you need to access an FDO animation icon. Dan Vrátil wrote: That's the thing: I want to feed the animation icon to KPixmapSequence, but KPixmapSequence only accepts full file path, or QPixmap. And I can't get the filename without depending on KIconTheme, which I cannot (I'm trying to add a widget to KWidgetsAddons), so the approach of getting a QPixmap from QIcon is the only way. Could you be more specific what kind of widget you plan to add to KWidgetAddons? I would really like to know if there is a way to solve this issue without potentially breaking users expecting to get square icons inside any sized rectangle. - Christoph --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120155/#review66332 --- On Sept. 12, 2014, 8:35 a.m., Dan Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120155/ --- (Updated Sept. 12, 2014, 8:35 a.m.) Review request for KDE Frameworks. Repository: kiconthemes Description --- KIconLoader (and KIconTheme) were always using single int to specify icon size, which implies only square icons. When requesting non-square icons however (such as pixmap sequence animations), the returned QPixmap is deformed. This patch ports all internals of KIconLoader to QSize. Diffs - src/kiconengine.cpp 530403e src/kiconloader.h 46d3017 src/kiconloader.cpp 2587b46 src/kicontheme.h ca04879 src/kicontheme.cpp 4f0e522 Diff: https://git.reviewboard.kde.org/r/120155/diff/ Testing --- QIcon icon = QIcon::fromTheme(QLatin1String(process-working)); QPixmap pix = icon.pixmap(22, 8 * 22); With this patch, the pix looks as expected. Thanks, Dan Vrátil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120160: KMessageWidget: fix handling of rapid succession of animatedHide+animatedShow calls
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/#review66869 --- src/kmessagewidget.cpp https://git.reviewboard.kde.org/r/120160/#comment46628 Would it make sense to emit hideAnimationFinished() before reversing the direction to showing? Likewise for the other case. - Christoph Feck On Sept. 18, 2014, 10:40 a.m., Fabio D'Urso wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/ --- (Updated Sept. 18, 2014, 10:40 a.m.) Review request for KDE Frameworks, Christoph Feck and Aurélien Gâteau. Repository: kwidgetsaddons Description --- Hi, I've found that if you call animatedShow() immediatly after animatedHide(), without waiting for the hide animation to finish first, the hide animation goes on and, in the end, the message widget becomes hidden. The attached patch adds checks in animatedShow and animatedHide so that, if an opposite animation is currently running, they just reverse it. Diffs - autotests/kmessagewidgetautotest.h 062e2b3 autotests/kmessagewidgetautotest.cpp f46bab0 src/kmessagewidget.cpp bc7ba32 tests/kmessagewidgettest.cpp f621b5a Diff: https://git.reviewboard.kde.org/r/120160/diff/ Testing --- I've added buttons to invoke animatedShow and animatedHide in tests/kmessagewidget.cpp and used them to test the behavior. I've also added an autotest that checks the final height and isVisible() value, both after single animatedShow/animatedHide calls and after animatedShow+animatedHide and animatedHide+animatedShow. Thanks, Fabio D'Urso ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120160: KMessageWidget: fix handling of rapid succession of animatedHide+animatedShow calls
On Sept. 18, 2014, 8:52 p.m., Christoph Feck wrote: src/kmessagewidget.cpp, line 412 https://git.reviewboard.kde.org/r/120160/diff/2/?file=313126#file313126line412 Would it make sense to emit hideAnimationFinished() before reversing the direction to showing? Likewise for the other case. Reading the previous reviews, it seems you already discussed it. - Christoph --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/#review66869 --- On Sept. 18, 2014, 10:40 a.m., Fabio D'Urso wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/ --- (Updated Sept. 18, 2014, 10:40 a.m.) Review request for KDE Frameworks, Christoph Feck and Aurélien Gâteau. Repository: kwidgetsaddons Description --- Hi, I've found that if you call animatedShow() immediatly after animatedHide(), without waiting for the hide animation to finish first, the hide animation goes on and, in the end, the message widget becomes hidden. The attached patch adds checks in animatedShow and animatedHide so that, if an opposite animation is currently running, they just reverse it. Diffs - autotests/kmessagewidgetautotest.h 062e2b3 autotests/kmessagewidgetautotest.cpp f46bab0 src/kmessagewidget.cpp bc7ba32 tests/kmessagewidgettest.cpp f621b5a Diff: https://git.reviewboard.kde.org/r/120160/diff/ Testing --- I've added buttons to invoke animatedShow and animatedHide in tests/kmessagewidget.cpp and used them to test the behavior. I've also added an autotest that checks the final height and isVisible() value, both after single animatedShow/animatedHide calls and after animatedShow+animatedHide and animatedHide+animatedShow. Thanks, Fabio D'Urso ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120160: KMessageWidget: fix handling of rapid succession of animatedHide+animatedShow calls
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/#review66871 --- I would really like to hear Aurelien's opinion, but maybe you can also explain: If a running hideAnimation is interrupted, and reversed by showing a different message, how does the widget handle the resize interruption? Will it resize smoothly during the animation? How does it handle the text change while it is visible? - Christoph Feck On Sept. 18, 2014, 10:40 a.m., Fabio D'Urso wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120160/ --- (Updated Sept. 18, 2014, 10:40 a.m.) Review request for KDE Frameworks, Christoph Feck and Aurélien Gâteau. Repository: kwidgetsaddons Description --- Hi, I've found that if you call animatedShow() immediatly after animatedHide(), without waiting for the hide animation to finish first, the hide animation goes on and, in the end, the message widget becomes hidden. The attached patch adds checks in animatedShow and animatedHide so that, if an opposite animation is currently running, they just reverse it. Diffs - autotests/kmessagewidgetautotest.h 062e2b3 autotests/kmessagewidgetautotest.cpp f46bab0 src/kmessagewidget.cpp bc7ba32 tests/kmessagewidgettest.cpp f621b5a Diff: https://git.reviewboard.kde.org/r/120160/diff/ Testing --- I've added buttons to invoke animatedShow and animatedHide in tests/kmessagewidget.cpp and used them to test the behavior. I've also added an autotest that checks the final height and isVisible() value, both after single animatedShow/animatedHide calls and after animatedShow+animatedHide and animatedHide+animatedShow. Thanks, Fabio D'Urso ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 120142: correct documentation for overlays parameter
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120142/ --- (Updated Sept. 18, 2014, 9:14 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kiconthemes Description --- correct documentation for overlays parameter The current documentation is incorrect, as no 'emblem' prefix is added to the overlay name. Also there is no mention how the emblems are placed. Instead of giving a complete definition, reference the drawOverlays method. Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de Diffs - src/kiconloader.h 46d3017e703c38bfa4da9e5cb721b0c596a26815 Diff: https://git.reviewboard.kde.org/r/120142/diff/ Testing --- Thanks, Stefan Brüns ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114933: KF5 Port of kdeui/kmessagewidgetdemo
On Aug. 12, 2014, 9:33 a.m., Aleix Pol Gonzalez wrote: After some discussion with the SDK and Book groups in the sprint, I think we should move this into the kde:kwidgetsaddons repository, in an examples subdirectory. Laurent Navet wrote: do you want me to move this from kdeexamples to kwidgetsaddons ? Christoph Feck wrote: While it indeed makes sense to showcase the features of a framework inside that framework, here I see the problem that the code currently depends on other frameworks. So either we showcase multiple frameworks, and have a separate examples repository, or we showcase a single framework, and make sure the examples do not drag in other frameworks. Has a decision been made to strip it down to Qt-only dependencies (and KWidgetAddons of course :) for adding it to KWidgetAddons, or ship it in a separate repository, where we can showcase it together with the other useful KF5 features, such as translations, kconfig etc.? - Christoph --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114933/#review64357 --- On Aug. 12, 2014, 9:33 a.m., Laurent Navet wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114933/ --- (Updated Aug. 12, 2014, 9:33 a.m.) Review request for KDE Examples, KDE Frameworks and Sune Vuorela. Repository: kdeexamples Description --- This is part of Google Code-IN Contest. As I'm no more student, I've waited for the end of the contest to work on it. Comments appreciated, Diffs - kdeui/kmessagewidgetdemo/CMakeLists.txt 12ef4ac kdeui/kmessagewidgetdemo/main.cpp d3a5bf0 kdeui/kmessagewidgetdemo/window.h d3a67c8 kdeui/kmessagewidgetdemo/window.cpp 9786da6 Diff: https://git.reviewboard.kde.org/r/114933/diff/ Testing --- Regression on KTextedit::setClickMessage(), as it don't exist in QTextEdit I've commented the line. Thanks, Laurent Navet ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119777: Make KFontUtils::adaptFontSize be a bit more exact
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119777/ --- (Updated Sept. 18, 2014, 9:43 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kguiaddons Description --- Not all fonts are linear in drawn size against their point size, the old code assumes so and simply guesses the next font size taking into account the pointSize we drawn, the geometry we got and the geometry we want. The new code is a bit slower but will make sure that we get better pointSize results in non linear fonts. This is a continuation from https://git.reviewboard.kde.org/r/114907/ Diffs - src/fonts/kfontutils.cpp 3a80039 Diff: https://git.reviewboard.kde.org/r/119777/diff/ Testing --- Blinken still works. Font sizes are similar. I did code an autotest to try situations like 9 and 10 like David suggested, problem is that i can't upload that test since it depends on the font, etc so that would be extremely unstable. Thanks, Albert Astals Cid ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
