D10989: Check for nullptr in indexForNode

2020-04-15 Thread Jaime Torres Amate
jtamate abandoned this revision. jtamate added a comment. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10989 To: jtamate, #frameworks, dfaure Cc: kde-frameworks-devel, mpyne, LeGast00n, cblack, michaelh, ngraham, bruns

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2019-07-21 Thread Jaime Torres Amate
jtamate closed this revision. jtamate added a comment. In D10742#499346 , @aacid wrote: > @jtamate Does that "let's continue in that other review" mean we should cancel this one? Still shows on the list of reviews to consider Yes, it

D14724: autotests: don't fail if an unrelated window shows up.

2019-04-01 Thread Jaime Torres Amate
jtamate accepted this revision. jtamate added a comment. This revision is now accepted and ready to land. As I don't have enough contiguous free time even to test it again, feel free to commit. You can add a comment to remember that it doesn't fix all the cases but is an improvement over

D15408: Don't assert deleting the temporary file

2019-03-05 Thread Jaime Torres Amate
jtamate planned changes to this revision. jtamate added a comment. The more I try to understand my own explanations, the less I agree with my past me. - Reading again the QTemporary documentation, without anything, the constructor creates a temporary file in the temp directory with

D19500: [KDirModel] Fix job urls change signal connection

2019-03-04 Thread Jaime Torres Amate
jtamate added a comment. +1. Just out of curiosity, Did you notice the problem because something was missing in execution? Unfortunately, neither the compiler nor I didn't noticed it. :-( REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D19500 To: broulik,

D17619: Change the path for every item of the subdirectories in a directory rename

2018-12-24 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R241:211c3a680ea7: Change the path for every item of the subdirectories in a directory rename (authored by jtamate). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D17619?vs=48048=48109#toc

D17619: Change the path for every item of the subdirectories in a directory rename

2018-12-23 Thread Jaime Torres Amate
jtamate updated this revision to Diff 48048. jtamate added a comment. With this version, the test with the patch applied is everlasting in: while [ true ]; do bin/kdirlistertest testRenameDirectory; if [ $? -ne 0 ]; then break; fi; done and is still crashing without the patch.

D17619: Change the path for every item of the subdirectories in a directory rename

2018-12-22 Thread Jaime Torres Amate
jtamate updated this revision to Diff 48004. jtamate marked 4 inline comments as done. jtamate added a comment. Changes requested done and reduced code duplication. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17619?vs=47995=48004 REVISION DETAIL

D17619: Change the path for every item of the subdirectories in a directory rename

2018-12-22 Thread Jaime Torres Amate
jtamate added inline comments. INLINE COMMENTS > dfaure wrote in kdirlistertest.cpp:1326 > What's drive_c? I started this patch following the structure of .wine/drive_c ... REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17619 To: jtamate, dfaure, #frameworks Cc:

D17619: Change the path for every item of the subdirectories in a directory rename

2018-12-22 Thread Jaime Torres Amate
jtamate updated this revision to Diff 47995. jtamate added a comment. Added the unit test. It crashes for me every time without the patch and none with the patch. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17619?vs=47731=47995 REVISION DETAIL

D17619: Change the path for every item of the subdirectories in a directory rename

2018-12-17 Thread Jaime Torres Amate
jtamate added a comment. I'll work on the unit test this weekend. I don't currently have enough free time on weekdays. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17619 To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham,

D17619: Change the path for every item of the subdirectories in a directory rename

2018-12-17 Thread Jaime Torres Amate
jtamate retitled this revision from "fix for bug 401552" to "Change the path for every item of the subdirectories in a directory rename". REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17619 To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel,

D17619: fix for bug 401552

2018-12-17 Thread Jaime Torres Amate
jtamate edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17619 To: jtamate, dfaure, #frameworks Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17619: fix for bug 401552

2018-12-17 Thread Jaime Torres Amate
jtamate updated this revision to Diff 47731. jtamate marked an inline comment as done. jtamate retitled this revision from "Unit test and fix for bug 401552" to "fix for bug 401552". jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a

D17619: Unit test and fix for bug 401552

2018-12-16 Thread Jaime Torres Amate
jtamate updated this revision to Diff 47666. jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a comment. I think this time I got the problem right. One of the classics: I was modifying the list while it was being used. REPOSITORY

D17619: Unit test and fix for bug 401552

2018-12-16 Thread Jaime Torres Amate
jtamate edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17619 To: jtamate, dfaure, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2018-12-16 Thread Jaime Torres Amate
jtamate added a comment. Let's continue on D17619 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D10742 To: jtamate, #frameworks, dfaure Cc: elvisangelaccio, bruns, kde-frameworks-devel, mwolff, markg, michaelh, ngraham

D17619: Unit test and fix for bug 401552

2018-12-16 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: dfaure, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY Add a unit test that fails before applying the patch and doesn't after the

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2018-12-15 Thread Jaime Torres Amate
jtamate added a comment. I'm able to reproduce the bug with the following steps: - Create a folder structure X/X1/X2/X3/X4 and add a new empty text file into each folder. - Within Dolphin, open a tab for each folder. - In the tab showing X contents, rename X1 to X1_ and the crash is

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2018-12-15 Thread Jaime Torres Amate
jtamate added a comment. I've been able to reproduce the bug: #10 0x7f44d257ae1d in qt_assert (assertion=assertion@entry=0x7f44d456fc43 "it != dirItem->lstItems.end()", file=file@entry=0x7f44d456fc10 "/g/5kde/frameworks/kio/src/core/kcoredirlister_p.h", line=line@entry=308) at

D16344: Do not try to fallback to "less secure" protocols

2018-10-26 Thread Jaime Torres Amate
jtamate added a comment. What protocol does KTcpSocket::SecureProtocols implement (I can't guess it)? If it is the same as QSsl:SecureProtocols it does: On the client side, this will send a TLS 1.0 Client Hello, enabling TLSv1_0 and SSLv3 connections. On

D16349: [kdirlistertest] Wait a little longer for the lister to finish

2018-10-22 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R241:87a2f084be83: [kdirlistertest] Wait a little longer for the lister to finish (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16349?vs=44011=44081

D16349: [kdirlistertest] Wait a little longer for the lister to finish

2018-10-21 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: dfaure, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY Wait a little longer and let m_dirLister finish before checking it has

D16072: Avoid waiting for user actions when kwin Focus stealing prevention is high or extreme

2018-10-14 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R241:63093179579c: Avoid waiting for user actions when kwin Focus stealing prevention is high or… (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D16072: Avoid waiting for user actions when kwin Focus stealing prevention is high or extreme

2018-10-14 Thread Jaime Torres Amate
jtamate added a comment. Created https://bugreports.qt.io/browse/QTBUG-71137 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D16072 To: jtamate, dfaure, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

kiowidgets_kdirmodeltest fail

2018-10-10 Thread Jaime
worked for me in the 4th revision of https://phabricator.kde.org/D10742 to kdirmodel.cpp around line 600, but without success. Any idea how to fix it is welcomed. Best Regards. Jaime.

D11282: less expensive findByUrl in KCoreDirListerCache

2018-10-10 Thread Jaime Torres Amate
jtamate abandoned this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11282 To: jtamate, #frameworks, dfaure Cc: kde-frameworks-devel, bruns, mwolff, michaelh, ngraham

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2018-10-09 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R241:20b89972b643: get rid of the raw KFileItem pointers in KCoreDirListerCache (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10742?vs=42345=43240

D16072: Avoid waiting for user actions when kwin Focus stealing prevention is high or extreme

2018-10-09 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: dfaure, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY When kwin focus stealing prevention is high or extreme, I had to activate

D16020: KFilePlacesModel: fix previous commit to avoid duplicating devices

2018-10-08 Thread Jaime Torres Amate
jtamate added a comment. Just in case, I'm subscribed now to the RSS https://build.kde.org/job/Frameworks/job/kio/rssFailed REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D16020 To: dfaure, jtamate Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15328: kfilewidget: convert connect syntax

2018-10-08 Thread Jaime Torres Amate
jtamate planned changes to this revision. jtamate added a comment. QTest::qWaitForWindowActive fails because I use kwin Focus stealing prevention High, therefore the windows doesn't become active until I click on them in the task bar or switch to them. And qWaitForWindowExposed doesn't

D15328: kfilewidget: convert connect syntax

2018-10-08 Thread Jaime Torres Amate
jtamate updated this revision to Diff 43106. jtamate added a comment. Fix a crash ,caused by a still connected signal, after running again the unittests. kfilewidgettest still doesn't pass because QTest::qWaitForWindowActive fails for me. Don't accept this revision until

D16020: KFilePlacesModel: fix previous commit to avoid duplicating devices

2018-10-08 Thread Jaime Torres Amate
jtamate accepted this revision. jtamate added a comment. This revision is now accepted and ready to land. I am really, really sorry. I have no excuses to not have run the autotests (that I usually run after my first fiasco). Hopefully, after my second fiasco, I'll run them before creating

D15890: kimg_rgb: optimize away QRegExp and QString::fromLocal8Bit.

2018-10-02 Thread Jaime Torres Amate
jtamate added inline comments. INLINE COMMENTS > rgb.cpp:702 > > -const QRegExp regexp(QLatin1String("^\x01\xda\x01[\x01\x02]")); > -QString data(QString::fromLocal8Bit(head)); > - > -return data.contains(regexp); > +return head.size() >= 4 && head.startsWith("\x01\xda\x01") &&

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

2018-09-26 Thread Jaime Torres Amate
jtamate updated this revision to Diff 42345. jtamate marked 5 inline comments as done. jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a comment. I've tried to do an automatic benchmark of the rename case, without success. Changed

D15740: Use non deprecated fastInsert in baloo

2018-09-25 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R293:a931be1d7e3a: Use non deprecated fastInsert in baloo (authored by jtamate). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15740?vs=42283=42315 REVISION DETAIL

D15635: Use String to store UDS_USER and UDS_GROUP of String type.

2018-09-25 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R293:95af521127c1: Use String to store UDS_USER and UDS_GROUP of String type. (authored by jtamate). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15635?vs=42044=42314

D15740: Use non deprecated fastInsert in baloo

2018-09-25 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: dfaure, Frameworks. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY Replace the deprecated uds insert method by fastInsert.

D15635: Use String to store UDS_USER and UDS_GROUP of String type.

2018-09-21 Thread Jaime Torres Amate
jtamate updated this revision to Diff 42044. jtamate retitled this revision from "Use String to store UDS_USER and UDS_GROUP of String type, use fastInsert." to "Use String to store UDS_USER and UDS_GROUP of String type.". jtamate edited the summary of this revision. jtamate edited the test plan

D15638: force-finish canberra notifications on close()

2018-09-21 Thread Jaime Torres Amate
jtamate accepted this revision. jtamate added a comment. This revision is now accepted and ready to land. Forget what I just wrote. I've seen that I was missing 15 lines in between then in the phabricator diff view. :-( Looks good to me. REPOSITORY R289 KNotifications BRANCH master

D15638: force-finish canberra notifications on close()

2018-09-21 Thread Jaime Torres Amate
jtamate added a comment. What would happen if finishNotification is called twice, In line 161 and then in line 192? My guess is that finished signal will be called twice with the same notification, and therefore KNotificationManager::notifyPluginFinished will be called twice. I don't

D15635: Use String to store UDS_USER and UDS_GROUP of String type, use fastInsert.

2018-09-21 Thread Jaime Torres Amate
jtamate updated this revision to Diff 42033. jtamate retitled this revision from "Use String to store UDS_USER and UDS_GROUP de tipo String" to "Use String to store UDS_USER and UDS_GROUP of String type, use fastInsert.". jtamate edited the summary of this revision. jtamate added a comment.

D15623: Finish the notification before removing it from the hash

2018-09-21 Thread Jaime Torres Amate
jtamate abandoned this revision. jtamate added a comment. I didn't realize the notifications are not owned by m_notifications :-(. Just looking at the backtrace of the crash, it was the only suspicious thing. REPOSITORY R289 KNotifications REVISION DETAIL

D15635: Use String to store UDS_USER and UDS_GROUP de tipo String

2018-09-21 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: dfaure, Baloo, Frameworks. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY First crash I get after enabling Dr. Konqi for slaves.

D15623: Finish the notification before removing it from the hash

2018-09-20 Thread Jaime Torres Amate
jtamate created this revision. jtamate added a reviewer: Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY If the notification is deleted from the hash, when the notification::id is

D15591: Add Open Document thumbnailer

2018-09-19 Thread Jaime Torres Amate
jtamate accepted this revision. jtamate added a comment. This revision is now accepted and ready to land. Ok by side. It installs now, and don't ask for passwords. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15591 To: broulik, #frameworks, #vdg, ngraham,

D15591: Add Open Document thumbnailer

2018-09-18 Thread Jaime Torres Amate
jtamate requested changes to this revision. jtamate added a comment. This revision now requires changes to proceed. I'm trying to check if it works with password-protected files, but I can't install it. The patch is missing opendocumentthumbnail.desktop. I have a question: If you have

D14072: Don't try to restore invalid user places

2018-09-18 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R241:2b52b47211a6: Dont try to restore invalid user places (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14072?vs=41879=41907 REVISION DETAIL

D13676: Make it possible to change directory up even with trailing slashes in the url

2018-09-18 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R241:6ab78df8c86e: Make it possible to change directory up even with trailing slashes in the url (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D15591: Add Open Document thumbnailer

2018-09-18 Thread Jaime Torres Amate
jtamate added a comment. I haven't tested it, therefore I ask: Does this thumbnailer ask for password for password protected files, like in https://bugs.kde.org/show_bug.cgi?id=394284 ? REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15591 To: broulik,

D14072: Don't try to restore invalid user places

2018-09-18 Thread Jaime Torres Amate
jtamate updated this revision to Diff 41879. jtamate added a comment. Continue processing the XML with invalid urls. And a trivial code deduplication, I couldn't resist. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14072?vs=37631=41879 REVISION DETAIL

D15068: Bindings: Correct handling of sources containing utf-8

2018-09-14 Thread Jaime Torres Amate
jtamate added a comment. Another solution (not tested here but used in other projects) could be to use with open(source, "r", encoding="utf-8") as f: (or if the file could contain the aberration BOM, you could use "utf-8-sig") And it should convert automatically all kind of line-ends

D15420: Intialize m_lastPosition

2018-09-11 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R39:3348145dbfbf: Intialize m_lastPosition (authored by jtamate). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15420?vs=41393=41419 REVISION DETAIL

D12016: [ktexteditor] much faster positionFromCursor

2018-09-11 Thread Jaime Torres Amate
jtamate added a comment. In D12016#323833 , @volkov wrote: > I get the following warning from valgrind because of this change: Fix in D15420 REPOSITORY R39 KTextEditor REVISION DETAIL

D15420: Intialize m_lastPosition

2018-09-11 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: KTextEditor, cullmann. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. jtamate requested review of this revision. REVISION SUMMARY Avoid uninitialized usage of m_lastPosition. TEST

D14724: autotests: don't fail if an unrelated window shows up.

2018-09-10 Thread Jaime Torres Amate
jtamate added a comment. It works for me if I use ctest -j6 . but fails for me if I use ctest -j12 . or ctest -j4 . 13/15 Test #3: plasma-dialogstatetest ...***Failed2.42 sec 3/15 Test #3: plasma-dialogstatetest ...***Failed1.71 sec REPOSITORY R242

D15408: Don't assert deleting the temporary file

2018-09-10 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: dfaure, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY Change the names of the temporary files to something more difficult for

D15328: kfilewidget: convert connect syntax

2018-09-09 Thread Jaime Torres Amate
jtamate updated this revision to Diff 41246. jtamate added a comment. Use nullptr instead of 0. Remove the copied/pasted part of the cause of two dissconnects. @bruns, as I'm not sure if the code emits other signals for lineEdit, I prefer to keep it as it was. REPOSITORY R241 KIO

D15328: kfilewidget: convert connect syntax

2018-09-07 Thread Jaime Torres Amate
jtamate updated this revision to Diff 41150. jtamate marked an inline comment as done. jtamate added a comment. Changed to 'Anonymous' connects and disconnects. Even if the documentation say: use 0 as a wildcard, the compiler thinks it should be a nullptr kfilewidget.cpp:1222:74:

D15328: kfilewidget: convert connect syntax

2018-09-07 Thread Jaime Torres Amate
jtamate marked an inline comment as done. jtamate added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kfilewidget.cpp:1225 > QObject::disconnect(m_connEditTextChanged); > m_connEditTextChanged = QMetaObject::Connection(); I'm sorry, but I still don't get it. Doesn't

D15328: kfilewidget: convert connect syntax

2018-09-07 Thread Jaime Torres Amate
jtamate added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kfilewidget.cpp:1225 > Whenever disconnect after that reset connection to QMetaObject::Connection > cause disconnect empty connection is safe, double disconnect same one - does > not. I'm sorry but I don't understand

D15328: kfilewidget: convert connect syntax

2018-09-07 Thread Jaime Torres Amate
jtamate updated this revision to Diff 41148. jtamate marked 2 inline comments as done. jtamate added a comment. Changed [&] by [this]in the lambdas. Added a lambda for KActionCollection::addAction. I didn't knew it was already possible. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D15328: kfilewidget: convert connect syntax

2018-09-07 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: dfaure, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY Convert from the old syntax to the new connect syntax TEST PLAN The

D15318: Automatically re-upload saved files located on samba shares instead of asking first

2018-09-06 Thread Jaime Torres Amate
jtamate added a comment. In D15318#321461 , @ngraham wrote: > > but wouldn't it be nicer to not get the dialog to overwrite the modified file > > You're still seeing that with git master? If so, I agree that we should fix that too.

D15318: Automatically re-upload saved files located on samba shares instead of asking first

2018-09-06 Thread Jaime Torres Amate
jtamate added a comment. It does automatically upload the modified file, but wouldn't it be nicer to not get the dialog to overwrite the modified file, that of course is not there in the case of local files. It is just a third parameter to copy and tell it to Overwrite. REPOSITORY R241

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-06 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. jtamate marked an inline comment as done. Closed by commit R241:820f622e86bb: kioexecd: watch for creations or modifications of the temporary files (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-06 Thread Jaime Torres Amate
jtamate marked an inline comment as done. jtamate added inline comments. INLINE COMMENTS > elvisangelaccio wrote in kioexecd.cpp:129 > Maybe `constBegin()`/`constEnd()` here? I always thought const iterators were meant to be used when the whole container is considered ReadOnly, not only its

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-05 Thread Jaime Torres Amate
jtamate updated this revision to Diff 41066. jtamate added a comment. Use QDateTime::currentDateTimeUtc() instead of QDateTime::currentDateTime(). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15180?vs=41035=41066 REVISION DETAIL

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-05 Thread Jaime Torres Amate
jtamate added a comment. May I commit or should I wait for @elvisangelaccio to accept the changes? This time I have read the arc message, that is usually something about non tracked files: Revision 'D15180 : kioexecd: watch for creations or

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-05 Thread Jaime Torres Amate
jtamate updated this revision to Diff 41035. jtamate added a comment. Tested, dirty is not signaled when created (at least I didn't saw the dialog for uploading the modified file). Removed the header. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-05 Thread Jaime Torres Amate
jtamate updated this revision to Diff 41030. jtamate added a comment. Why the mutex? I interpreted that QTimer work as an interrumpt (wrong) instead of generating events in the event queue (right). So I've gone through all the possible mistakes one can do here: - Design mistakes -

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-05 Thread Jaime Torres Amate
jtamate updated this revision to Diff 41026. jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a comment. Do not delete recursively. Do not delete the file after 30s (we know it is already deleted). REPOSITORY R241 KIO CHANGES

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-05 Thread Jaime Torres Amate
jtamate added inline comments. INLINE COMMENTS > elvisangelaccio wrote in kioexecd.cpp:65 > The problem with using `QDir::removeRecursively()` is that the folder we are > going to delete recursively is an input from dbus. What happens if some > malicious software calls `watch("~/dummy.txt")` ?

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-04 Thread Jaime Torres Amate
jtamate updated this revision to Diff 40961. jtamate added a comment. I missed that part, sorry. It is safer that way, the start(time) method was added in Qt 5.8. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15180?vs=40960=40961 REVISION DETAIL

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-04 Thread Jaime Torres Amate
jtamate updated this revision to Diff 40960. jtamate marked 3 inline comments as done. jtamate added a comment. Done. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15180?vs=40952=40960 REVISION DETAIL https://phabricator.kde.org/D15180 AFFECTED FILES

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-04 Thread Jaime Torres Amate
jtamate updated this revision to Diff 40952. jtamate marked 4 inline comments as done. jtamate added a comment. Implemented Anthony comments/suggestions. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15180?vs=40908=40952 REVISION DETAIL

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-03 Thread Jaime Torres Amate
jtamate updated this revision to Diff 40908. jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a comment. As I'm never sure if a timed execution can happen in the middle of other execution, I've added a mutex for m_deleted. REPOSITORY

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-03 Thread Jaime Torres Amate
jtamate added a comment. In D15180#319314 , @anthonyfieroni wrote: > I'm unhappy with that stop watching is on exit == 0, so when it's not, somehow, containers will continue to grow, it'll result in higher memory usage and slower performance.

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-03 Thread Jaime Torres Amate
jtamate updated this revision to Diff 40888. jtamate added a reviewer: elvisangelaccio. jtamate added a comment. Updated with the code that handles m_openedBy right. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15180?vs=40779=40888 REVISION DETAIL

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-09-02 Thread Jaime Torres Amate
jtamate edited the summary of this revision. jtamate edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D15180 To: jtamate, #frameworks, broulik, ngraham, dfaure Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-08-31 Thread Jaime Torres Amate
jtamate added inline comments. INLINE COMMENTS > broulik wrote in kioexecd.cpp:58 > If you can be sure kioexec creates a folder per temp file (it might does), > then this is probably fine. > Also check kioexec what it does when the process closes, if it also > recursively removes the temp dir

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-08-31 Thread Jaime Torres Amate
jtamate updated this revision to Diff 40779. jtamate marked 3 inline comments as done. jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a comment. I'm not sure if I have to keep compatibility in the dbus calls, therefore the old one is

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-08-31 Thread Jaime Torres Amate
jtamate marked 2 inline comments as done. jtamate added inline comments. INLINE COMMENTS > broulik wrote in kioexecd.cpp:58 > I'm not sure that's a good idea Why not? It should be just removing the temporary file copy and it's backups. > broulik wrote in kioexecd.cpp:73 > You never unwatch the

D15180: kioexecd: watch for creations or modifications of the temporary files

2018-08-31 Thread Jaime Torres Amate
jtamate created this revision. jtamate added reviewers: Frameworks, broulik, ngraham, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY When a non KIO friendly program opens a non local file,

D11604: kdirlistertest doesn't fail at random

2018-08-19 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R241:0c8cd4076b7d: kdirlistertest doesnt fail at random (authored by jtamate). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11604?vs=39991=3 REVISION DETAIL

D11604: kdirlistertest doesn't fail at random

2018-08-19 Thread Jaime Torres Amate
jtamate updated this revision to Diff 39991. jtamate marked an inline comment as done. jtamate added a comment. I'm never sure at how many lines the comments affects, but the last one should be: QTRY_COMPARE(m_dirLister.spyClear.count(), 1);

D11604: kdirlistertest doesn't fail at random

2018-08-19 Thread Jaime Torres Amate
jtamate updated this revision to Diff 39990. jtamate marked 11 inline comments as done. jtamate added a comment. Fixed the inline comments. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11604?vs=39836=39990 REVISION DETAIL

D13813: Make this test work again with new uds implementation

2018-08-16 Thread Jaime Torres Amate
jtamate added a comment. In D13813#310047 , @cfeck wrote: > Authored by a Bot? Somehow my name and email address in .gitconfig got changed. How? No idea. Should revert and push again with the right name? REPOSITORY R318 Dolphin

D11604: kdirlistertest doesn't fail at random

2018-08-16 Thread Jaime Torres Amate
jtamate updated this revision to Diff 39836. jtamate marked 3 inline comments as done. jtamate added a comment. Restored all the signalSpy wait wrongly deleted. Removed the use of qmimedatabase. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D11604: kdirlistertest doesn't fail at random

2018-08-15 Thread Jaime Torres Amate
jtamate updated this revision to Diff 39801. jtamate edited the summary of this revision. jtamate added a comment. Herald added a subscriber: kde-frameworks-devel. Removed the use of enterLoop and exitLoop in favor of QTRY_COMPARE and QTRY_VERIFY. Removed some waits Changed all the

D14859: Don't try to call transaction documentUrl with a 0 id

2018-08-15 Thread Jaime Torres Amate
jtamate updated this revision to Diff 39788. jtamate added a comment. Done requested changes. REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14859?vs=39784=39788 REVISION DETAIL https://phabricator.kde.org/D14859 AFFECTED FILES

D14859: Don't try to call transaction documentUrl with a 0 id

2018-08-15 Thread Jaime Torres Amate
jtamate created this revision. jtamate added a reviewer: Frameworks. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. jtamate requested review of this revision. REVISION SUMMARY In Transaction::documentUrl(quint64 id) const, there is Q_ASSERT(id

D13813: Make this test work again with new uds implementation

2018-08-15 Thread Jaime Torres Amate
jtamate added a comment. In D13813#309371 , @elvisangelaccio wrote: > @jtamate Any updates on this? Can you use the new `fastInsert()` calls here? Not until dolphin depends on KIO 5.47, currently its minimum required kio version is

D12945: kcoredirlister lstItems benchmark

2018-07-27 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R241:908bfca9fe72: kcoredirlister lstItems benchmark (authored by jtamate). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D12945?vs=38134=38612#toc REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D12945: kcoredirlister lstItems benchmark

2018-07-27 Thread Jaime Torres Amate
jtamate edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12945 To: jtamate, dfaure, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14302: Don't block forever in ensureKdeinitRunning

2018-07-26 Thread Jaime Torres Amate
jtamate added a comment. Qt 5.11.0 and frameworks 5.46.0 over xcb. I'm sorry, I'll have to wait until the next lock, I didn't find the lock file neither the blocked kdeinit5. Is the complete name of the lock file be logged with qCDebug() ?. Just after killing dolphin yesterday afternoon

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes. Closed by commit R271:8097e1aa0d21: Dont block forever in ensureKdeinitRunning (authored by jtamate). REPOSITORY R271 KDBusAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14302?vs=38403=38448 REVISION

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38403. jtamate edited the summary of this revision. jtamate added a comment. Don't wait forever, if the lock is blocked for more than 30 seconds, write a warning and don't try to start kdeinit5 again. REPOSITORY R271 KDBusAddons CHANGES SINCE LAST

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38387. jtamate edited the summary of this revision. jtamate added a comment. > I said from the start that it wasn't tryLock() that was blocking but lock(), good to see that this is now confirmed. I trusted blindly in gdb stacktrace, and also I didn't

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38372. jtamate edited the summary of this revision. jtamate edited the test plan for this revision. jtamate added a comment. It is my first time of a backtrace that doesn't reflect the true state of a program. But I've learned the lesson, in case of

  1   2   3   4   5   >