Re: KDateTimeEdit is broken (but unused)
On Saturday 18 April 2015 23:21:59 David Faure wrote: In a KDateTimeEdit, changing the date in the date combo (whether by typing or selecting e.g. next month in the popupmenu), does NOT change the value of date(), which uses its own member variable, unaffected by user interaction. [...] This gives two options: 1) fixing KDateTimeEdit 2) deprecating KDateTimeEdit Does https://paste.kde.org/pksfgjntf fix it? I would prefer fixing it over option 2. Christoph Feck (kdepepo) ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121599: Fix crash in KIO due to exposing inconsistent views of internal data.
On April 19, 2015, 4:27 a.m., Simeon Bird wrote: Hi everyone, Sorry to bother you, but may I please commit this? It's been a while. The kdelibs version had some positive reviews. Thanks, Simeon Albert Astals Cid wrote: Any reason why we should not fix this in kdelibs too? Not at all, the patch applies fairly cleanly. I just didn't realise there were going to be more releases of kdelibs. - Simeon --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121599/#review79194 --- On Feb. 28, 2015, 10:49 p.m., Simeon Bird wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121599/ --- (Updated Feb. 28, 2015, 10:49 p.m.) Review request for KDE Frameworks, Aleix Pol Gonzalez and David Faure. Repository: kio 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. This is the same as reviewboard 117345, but on the KF5 kio repository and with a test case. While I was doing this I also fixed a minor bug in the test suite (which is a separate commit). Diffs - autotests/kdirlistertest.h 70fd16f autotests/kdirlistertest.cpp b588e5b src/core/kcoredirlister.cpp f5c5627 Diff: https://git.reviewboard.kde.org/r/121599/diff/ Testing --- Tests run Thanks, Simeon Bird ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121599: Fix crash in KIO due to exposing inconsistent views of internal data.
On April 19, 2015, 4:27 a.m., Simeon Bird wrote: Hi everyone, Sorry to bother you, but may I please commit this? It's been a while. The kdelibs version had some positive reviews. Thanks, Simeon Albert Astals Cid wrote: Any reason why we should not fix this in kdelibs too? Simeon Bird wrote: Not at all, the patch applies fairly cleanly. I just didn't realise there were going to be more releases of kdelibs. Albert Astals Cid wrote: Do you read the emails i send to kde-cvs-annou...@kde.org ? I think i've been clearly direct in stating that we're having LTS releases of kdelibs(KDE/4.14), kdepim(KDE/4.14), kdepimlibs(KDE/4.14), kdepim- runtime(KDE/4.14) and kde-workspace (KDE/4.11) but since you did not know it, i guess i've failed there. Could you please tell me what can i do to improve it so you would know we are still dooing kdelibs releases? I hadn't seen kdelibs in the 15.04 emails so I thought 14.12 was the last one. - Simeon --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121599/#review79194 --- On Feb. 28, 2015, 10:49 p.m., Simeon Bird wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121599/ --- (Updated Feb. 28, 2015, 10:49 p.m.) Review request for KDE Frameworks, Aleix Pol Gonzalez and David Faure. Repository: kio 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. This is the same as reviewboard 117345, but on the KF5 kio repository and with a test case. While I was doing this I also fixed a minor bug in the test suite (which is a separate commit). Diffs - autotests/kdirlistertest.h 70fd16f autotests/kdirlistertest.cpp b588e5b src/core/kcoredirlister.cpp f5c5627 Diff: https://git.reviewboard.kde.org/r/121599/diff/ Testing --- Tests run Thanks, Simeon Bird ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121599: Fix crash in KIO due to exposing inconsistent views of internal data.
On abr. 19, 2015, 4:27 a.m., Simeon Bird wrote: Hi everyone, Sorry to bother you, but may I please commit this? It's been a while. The kdelibs version had some positive reviews. Thanks, Simeon Albert Astals Cid wrote: Any reason why we should not fix this in kdelibs too? Simeon Bird wrote: Not at all, the patch applies fairly cleanly. I just didn't realise there were going to be more releases of kdelibs. Do you read the emails i send to kde-cvs-annou...@kde.org ? I think i've been clearly direct in stating that we're having LTS releases of kdelibs(KDE/4.14), kdepim(KDE/4.14), kdepimlibs(KDE/4.14), kdepim- runtime(KDE/4.14) and kde-workspace (KDE/4.11) but since you did not know it, i guess i've failed there. Could you please tell me what can i do to improve it so you would know we are still dooing kdelibs releases? - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121599/#review79194 --- On feb. 28, 2015, 10:49 p.m., Simeon Bird wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121599/ --- (Updated feb. 28, 2015, 10:49 p.m.) Review request for KDE Frameworks, Aleix Pol Gonzalez and David Faure. Repository: kio 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. This is the same as reviewboard 117345, but on the KF5 kio repository and with a test case. While I was doing this I also fixed a minor bug in the test suite (which is a separate commit). Diffs - autotests/kdirlistertest.h 70fd16f autotests/kdirlistertest.cpp b588e5b src/core/kcoredirlister.cpp f5c5627 Diff: https://git.reviewboard.kde.org/r/121599/diff/ Testing --- Tests run Thanks, Simeon Bird ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123341: Optimize reading Sonnet settings by minimizing the cals to save()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123341/ --- (Updated April 19, 2015, 7:46 p.m.) Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. Changes --- Added unit tests: -Check that there are no unneeded writes to the settings. (Does not work on Windows, but does not fail) -Check that the settings changes are written to QSettings imediately from Speller functions. Repository: sonnet Description --- The curent implementation can causes ~25 cals to Settings::save() for every created Speller instance. The Settings::restore() function ends with setQuietIgnoreList(...) which would call save() for evey ignore-word. This patch removes the save() cals in the setFoo() functions of Settings and replace it by a boolean that indicates if the settign is changed or not. Then in the Speller class save() is called when needed. The reason for this patch is that it took Kate ~2 minutes to open a kate session with ~340 files. The implementation in Kate called reload() for every file. There is a RR for Kate to remove that unneeded reload. This patch also prepares Sonnet::Settings for being used in the public API to set aplication specific Sonnet settings without saving them in the global settings. The Settings class is exported but the header is private. This change is BIC, but since the class is private, my interpretation is that it should not mater. If accepted I will also add a Review Request to make the Settings class public to enable application specific settings. Diffs (updated) - autotests/CMakeLists.txt 8ade413 src/core/settings.cpp b5c4198 src/core/settings_p.h 0d48889 src/core/speller.cpp 3903b42 src/ui/configwidget.cpp 02f2a26 Diff: https://git.reviewboard.kde.org/r/123341/diff/ Testing (updated) --- Tested with Kate and with the patch the startup time was back to normal. Two unit tests added. Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123341: Optimize reading Sonnet settings by minimizing the cals to save()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123341/ --- (Updated April 19, 2015, 7:49 p.m.) Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. Changes --- And add the new test files... Repository: sonnet Description --- The curent implementation can causes ~25 cals to Settings::save() for every created Speller instance. The Settings::restore() function ends with setQuietIgnoreList(...) which would call save() for evey ignore-word. This patch removes the save() cals in the setFoo() functions of Settings and replace it by a boolean that indicates if the settign is changed or not. Then in the Speller class save() is called when needed. The reason for this patch is that it took Kate ~2 minutes to open a kate session with ~340 files. The implementation in Kate called reload() for every file. There is a RR for Kate to remove that unneeded reload. This patch also prepares Sonnet::Settings for being used in the public API to set aplication specific Sonnet settings without saving them in the global settings. The Settings class is exported but the header is private. This change is BIC, but since the class is private, my interpretation is that it should not mater. If accepted I will also add a Review Request to make the Settings class public to enable application specific settings. Diffs (updated) - autotests/CMakeLists.txt 8ade413 autotests/test_settings.h PRE-CREATION autotests/test_settings.cpp PRE-CREATION src/core/settings.cpp b5c4198 src/core/settings_p.h 0d48889 src/core/speller.cpp 3903b42 src/ui/configwidget.cpp 02f2a26 Diff: https://git.reviewboard.kde.org/r/123341/diff/ Testing --- Tested with Kate and with the patch the startup time was back to normal. Two unit tests added. Thanks, Kåre Särs ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121086: Rename jpegcreatorsettings.kcfg to avoid conflicts with KDE4
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121086/ --- (Updated April 19, 2015, 7:20 p.m.) Status -- This change has been marked as submitted. Review request for KDE Runtime, KDE Frameworks and Plasma. Repository: kio-extras Description --- Simply add 5 as a suffix to avoid conflicts with KDE 4 version which is shipped in KDE-Runtime which isn't going away any time now. Diffs - thumbnail/CMakeLists.txt e9ab79b thumbnail/jpegcreator.cpp de4902b thumbnail/jpegcreatorsettings.kcfg 8f68f46 thumbnail/jpegcreatorsettings.kcfgc 3f6cdab thumbnail/jpegcreatorsettings5.kcfg PRE-CREATION thumbnail/jpegcreatorsettings5.kcfgc PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121086/diff/ Testing --- Thanks, Armin K. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
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 QListcustom type 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 QListQStorageInfo 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? There is some historic reference here: http://marc.info/?l=kde-core-develm=113069150408826w=2 MSVC 2013 still has the same beahviour. - Andrius da Costa --- 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-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 123436: Add unit test for KIO::AccessManager
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123436/ --- Review request for KDE Frameworks. Repository: kio Description --- I was having some weird behavior I wanted to test, having a test was a minimum. Diffs - autotests/CMakeLists.txt 0a9fcea autotests/accessmanagertest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123436/diff/ Testing --- The test passes as is. Served me to fix: https://codereview.qt-project.org/#/c/110745/ Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123436: Add unit test for KIO::AccessManager
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123436/ --- (Updated April 20, 2015, 3:12 a.m.) Review request for KDE Frameworks. Repository: kio Description --- I was having some weird behavior I wanted to test, having a test was a minimum. Diffs - autotests/CMakeLists.txt 0a9fcea autotests/accessmanagertest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123436/diff/ Testing (updated) --- The test passes as is. Served me to fix: https://git.reviewboard.kde.org/r/123325/ https://codereview.qt-project.org/#/c/110745/ Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123417: Prevent plasmashell from crashing on wayland
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/#review79237 --- Approach looks correct to me, but can be simplified (see below). A general comment: please watch the coding style, e.g. we use four spaces for indentation. src/kidletime.cpp (lines 193 - 194) https://git.reviewboard.kde.org/r/123417/#comment54137 suggestion: merge the two ifs with an . If isPlatformX11 is false, the other condition isn't checked, so it's save src/kidletime.cpp (line 240) https://git.reviewboard.kde.org/r/123417/#comment54138 I don't think that this check is needed. Trying to cast should not matter. - Martin Gräßlin On April 18, 2015, 7:19 p.m., Nerdopolis Turfwalker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123417/ --- (Updated April 18, 2015, 7:19 p.m.) Review request for KDE Frameworks. Repository: kidletime Description --- kidletime makes an unchecked X call, which is used by plasmashell. Diffs - CMakeLists.txt 5335bd5db171721635517a82829f54cd1b3f6326 src/config-kidletime.h.cmake d3f0cfd78833d2b3acf90d0a068905676ceba586 src/kidletime.cpp f38ae467d78d899b70b602a932482a39402793fb Diff: https://git.reviewboard.kde.org/r/123417/diff/ Testing --- Ran plasmashell with QT_QPA_PLATFORM=wayland, and got no more crashes Thanks, Nerdopolis Turfwalker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123419: CodingStyle for autotests
On April 18, 2015, 10:04 p.m., David Faure wrote: Looks ok but it's strange to quote a wiki page called *kdepim*. This isn't kdepim. You are right. I continue the work I do for kdepim. Where should I put the informations? - Guy --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123419/#review79179 --- On April 19, 2015, 4:52 p.m., Guy Maurel wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123419/ --- (Updated April 19, 2015, 4:52 p.m.) Review request for KDE Frameworks, Mario Bensi and David Faure. Repository: karchive Description --- Details can be seen at: http://techbase.kde.org/Policies/Kdepim_Coding_Style Diffs - autotests/karchivetest.cpp 9e27ffcedbd8a6b62ee080238713580f2a81eb07 autotests/kfiltertest.cpp e0d9c57 autotests/klimitediodevicetest.h 6ec205f Diff: https://git.reviewboard.kde.org/r/123419/diff/ Testing --- Thanks, Guy Maurel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KDateTimeEdit is broken (but unused)
On Sunday 19 April 2015 09:54:14 Christoph Feck wrote: On Saturday 18 April 2015 23:21:59 David Faure wrote: In a KDateTimeEdit, changing the date in the date combo (whether by typing or selecting e.g. next month in the popupmenu), does NOT change the value of date(), which uses its own member variable, unaffected by user interaction. [...] This gives two options: 1) fixing KDateTimeEdit 2) deprecating KDateTimeEdit Does https://paste.kde.org/pksfgjntf fix it? It helps. The unittest passes. But note how KDateTimeEdit has 3 signals dateTimeEntered, dateTimeChanged and dateTimeEdited. Calling setDate only emits dateTimeChanged. I pushed the test and the fix - and debug statements in kdatetimeedittestapp showing which signals get emitted. Feel free to use this to fix it further :-) I would prefer fixing it over option 2. Well I was wondering how useful it was, if nobody had been using it in 4 years ;-) But OK I guess it's a step in the direction of allowing other calendar systems in KDE apps, it's just that nobody actually used that possibility in their apps -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123419: CodingStyle for autotests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123419/ --- (Updated April 19, 2015, 2:52 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Mario Bensi and David Faure. Changes --- Submitted with commit b8f25b58e7dbf641fbe5e16c2982566c6729e9de by Guy Maurel to branch master. Repository: karchive Description --- Details can be seen at: http://techbase.kde.org/Policies/Kdepim_Coding_Style Diffs - autotests/karchivetest.cpp 9e27ffcedbd8a6b62ee080238713580f2a81eb07 autotests/kfiltertest.cpp e0d9c57 autotests/klimitediodevicetest.h 6ec205f Diff: https://git.reviewboard.kde.org/r/123419/diff/ Testing --- Thanks, Guy Maurel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121079: Fix building dolphin tests on MSVC
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121079/ --- (Updated Abril 19, 2015, 3:13 p.m.) Status -- This change has been discarded. Review request for KDE Base Apps, KDE Frameworks and kdewin. Repository: kde-baseapps Description --- MSVC complains about missing symbols from their respective moc files on linking step. Diffs - dolphin/src/tests/CMakeLists.txt 60f6517 Diff: https://git.reviewboard.kde.org/r/121079/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-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121079: Fix building dolphin tests on MSVC
On Fev. 12, 2015, 10:54 p.m., Albert Astals Cid wrote: Patch doesn't apply, please rebase superseded by RR https://git.reviewboard.kde.org/r/123270/, discarding this one. - Andrius da Costa --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121079/#review75951 --- 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/121079/ --- (Updated Nov. 8, 2014, 10:26 p.m.) Review request for KDE Base Apps, KDE Frameworks and kdewin. Repository: kde-baseapps Description --- MSVC complains about missing symbols from their respective moc files on linking step. Diffs - dolphin/src/tests/CMakeLists.txt 60f6517 Diff: https://git.reviewboard.kde.org/r/121079/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-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123419: CodingStyle for autotests
On April 18, 2015, 8:04 p.m., David Faure wrote: Looks ok but it's strange to quote a wiki page called *kdepim*. This isn't kdepim. Guy Maurel wrote: You are right. I continue the work I do for kdepim. Where should I put the informations? rename the page to Scripts for applying the kdelibs coding style or something like that? - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123419/#review79179 --- On April 19, 2015, 2:52 p.m., Guy Maurel wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123419/ --- (Updated April 19, 2015, 2:52 p.m.) Review request for KDE Frameworks, Mario Bensi and David Faure. Repository: karchive Description --- Details can be seen at: http://techbase.kde.org/Policies/Kdepim_Coding_Style Diffs - autotests/karchivetest.cpp 9e27ffcedbd8a6b62ee080238713580f2a81eb07 autotests/kfiltertest.cpp e0d9c57 autotests/klimitediodevicetest.h 6ec205f Diff: https://git.reviewboard.kde.org/r/123419/diff/ Testing --- Thanks, Guy Maurel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121599: Fix crash in KIO due to exposing inconsistent views of internal data.
On abr. 19, 2015, 4:27 a.m., Simeon Bird wrote: Hi everyone, Sorry to bother you, but may I please commit this? It's been a while. The kdelibs version had some positive reviews. Thanks, Simeon Any reason why we should not fix this in kdelibs too? - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121599/#review79194 --- On feb. 28, 2015, 10:49 p.m., Simeon Bird wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121599/ --- (Updated feb. 28, 2015, 10:49 p.m.) Review request for KDE Frameworks, Aleix Pol Gonzalez and David Faure. Repository: kio 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. This is the same as reviewboard 117345, but on the KF5 kio repository and with a test case. While I was doing this I also fixed a minor bug in the test suite (which is a separate commit). Diffs - autotests/kdirlistertest.h 70fd16f autotests/kdirlistertest.cpp b588e5b src/core/kcoredirlister.cpp f5c5627 Diff: https://git.reviewboard.kde.org/r/121599/diff/ Testing --- Tests run Thanks, Simeon Bird ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
