D10989: Check for nullptr in indexForNode

[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

[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 should be closed. It was already commited, unfortunately with a bug 
associated to it, fixed in the other review.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10742

To: jtamate, #frameworks, dfaure
Cc: aacid, elvisangelaccio, bruns, kde-frameworks-devel, mwolff, markg, 
LeGast00n, sbergeron, michaelh, ngraham

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

[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 previous situation.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D14724

To: dfaure, hein, drosca, broulik, davidedmundson, jtamate
Cc: jtamate, kde-frameworks-devel, #plasma, michaelh, ngraham, bruns

D15408: Don't assert deleting the temporary file

[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 prefix the 
application name.
- Why would the temporary file be deleted if the trash is being emptied?
  - Unless KIO::file_copy closes the file if the origin file to be copied 
is not available. If it is the case, a comment is needed.
  
  Also the asserts were modified by commit 
7ce34436a0387f3620e3f47ea7252d151e839776 


REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15408

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19500: [KDirModel] Fix job urls change signal connection

[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, #frameworks, dfaure, jtamate
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

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

[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

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17619?vs=48048=48109

REVISION DETAIL
  https://phabricator.kde.org/D17619

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h
  src/core/kcoredirlister.cpp

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

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

[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.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17619?vs=48004=48048

REVISION DETAIL
  https://phabricator.kde.org/D17619

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h
  src/core/kcoredirlister.cpp

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

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

[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
  https://phabricator.kde.org/D17619

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h
  src/core/kcoredirlister.cpp

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

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

[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: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

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

[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
  https://phabricator.kde.org/D17619

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h
  src/core/kcoredirlister.cpp

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

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

[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, bruns

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

[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, michaelh, ngraham, bruns

D17619: fix for bug 401552

[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

[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 comment.


  There is no need for a unit test, it is already in kdirmodeltest (but perhaps 
could be expanded in another patch).
  
  I've realized that it only modifies all paths of the subdirectories, and 
therefore if all the items of a sorted list change the same values, the order 
is preserved.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17619?vs=47666=47731

REVISION DETAIL
  https://phabricator.kde.org/D17619

AFFECTED FILES
  src/core/kcoredirlister.cpp

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17619: Unit test and fix for bug 401552

[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
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17619?vs=47655=47666

REVISION DETAIL
  https://phabricator.kde.org/D17619

AFFECTED FILES
  src/core/kcoredirlister.cpp

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17619: Unit test and fix for bug 401552

[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

[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

[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 
patch (in skeleton only)
  
  Could someone that was able to reproduce the crash can test the patch to see 
if it really fixes the crash?

TEST PLAN
  Fail in the unit test before applying the patch
  Doesn't fail after applying the patch.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D17619

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h
  src/core/kcoredirlister.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

[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 there.
  
  Next step, transform this into a unit test that always crashes.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10742

To: jtamate, #frameworks, dfaure
Cc: elvisangelaccio, bruns, kde-frameworks-devel, mwolff, markg, michaelh, 
ngraham

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

[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 
../../include/QtCore/../../src/corelib/global/qlogging.h:91
#11 0x7f44d4535c7a in KCoreDirListerCache::reinsert 
(this=this@entry=0x7f44d45a5440 <(anonymous 
namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, item=..., 
oldUrl=...) at /g/5kde/frameworks/kio/src/core/kcoredirlister_p.h:310
#12 0x7f44d452825c in KCoreDirListerCache::renameDir 
(this=this@entry=0x7f44d45a5440 <(anonymous 
namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, oldUrl=..., 
newUrl=...) at /g/5kde/frameworks/kio/src/core/kcoredirlister.cpp:1584
#13 0x7f44d452aa29 in KCoreDirListerCache::slotFileRenamed 
(this=0x7f44d45a5440 <(anonymous 
namespace)::Q_QGS_kDirListerCache::innerFunction()::holder>, _src=..., 
_dst=..., dstPath=...) at /g/5kde/frameworks/kio/src/core/kcoredirlister.cpp:969
  
  Let's see if this assert/crash can be avoided without reverting all the 
patch. Any help is welcome.

INLINE COMMENTS

> dfaure wrote in kcoredirlister.cpp:963
> Where did the old code call refresh() (which you now call inside findByUrl)? 
> I don't see it, I only see more specific calls in more specific cases. So 
> this looks slower and possibly incorrect (for non-local-files).

Changed to use the reinsert semantic.

> dfaure wrote in kcoredirlister.cpp:1004
> This used to modify the fileitem in dirItem->lstItems, now it's modifying a 
> copy.
> Same for all other fileitem.setFoo calls below.
> 
> Is there a call to reinsert missing?

Thanks, reinsert has a better semantics than refresh.

> markg wrote in kcoredirlister.cpp:825-829
> Hmm, this looks weird to me.
> Sure, it works. "KFileItem retKFileItem = *it;" makes a copy.
> 
> A more efficient way (but requires you to change the lists this is backed by 
> to a std::vector) is to:
> 
> 1. Take the element out of the vector. Something like "KFileItem item = 
> std::move(*it);
> 2. Now the vector would be in a valid state but with one invalid object (will 
> post a problem if you iterate over it later on) so you have to remove that 
> element from the vector like you did.
> 
> I'm not sure if the benefits of this justify the path of changing the list 
> (dirItem->lstItems) to a std::vector, if possible at all.
> That's up to you :)

Changed, using the reinsert semantic.

> bruns wrote in kcoredirlister_p.h:302
> This can be better implemented with std::move and std::move_backward from 
> 
> http://www.cplusplus.com/reference/algorithm/move/
> 
> 1. calculate start and end iterators from the old and new URL
> 2. move old item to tmp
> 3.
> 
>   if (old_it < new_it) {
>  std::move(old_it + 1, new_it, old_it); // move downwards
>   } else {
>  std::move_backward(new_it , old_it - 1, old_it);
>   }
>   *new_it = tmp;

I've looked at those methods, they create a new "undefined" status in the list. 
I don't want to worry about this new status.

> bruns wrote in kfileitem.cpp:180
> You can probably leave this out if you use the following for ordering:
> 
>   bool operator< (const KFileItem& other) {
> if m_url.size() != other.m_url.size() {
>   return m_url.size() < other.m_url.size()
> }
> return m_url < other.m_url;
>   }
> 
> You don't need lexicographical sorting, but just a total ordering for the 
> lookup.

I've tried something similar. As QUrl doesn't have a size method, I've tried 
with path().size(). With findByUrl it is no problem, it gives me 28 msecs vs. 
1795 msecs, but inserting in the list gives me 37 msecs vs. the original 19 
msecs. 
Only checking m_url < other.m_url (without m_hash) gives me 21 msecs inserting 
and 15 msecs in findByUrl, faster than using m_hash with the right checks for 
collisions (22-23 msecs inserting).

> bruns wrote in kfileitem.h:490
> The description does not match the implementation ...

Changed, thanks. Now they match.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10742

To: jtamate, #frameworks, dfaure
Cc: elvisangelaccio, bruns, kde-frameworks-devel, mwolff, markg, michaelh, 
ngraham

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

[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 the server side, this will enable both SSLv3 and 
TLSv1_0 connections.
  
  Shouldn't it try with TLS 1.3 when available and fall back to TLS 1.2, but 
not lower (for security reason)?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D16344

To: aacid
Cc: jtamate, carewolf, dfaure, stikonas, kde-frameworks-devel, michaelh, 
ngraham, bruns

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

[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

REVISION DETAIL
  https://phabricator.kde.org/D16349

AFFECTED FILES
  autotests/kdirlistertest.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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 
finished.

TEST PLAN
  Before:
  FAIL!  : KDirListerTest::testConcurrentListingAndStop() Compared values are 
not the same
  
Actual   (m_dirLister.spyCompleted.count()): 0
Expected (1)   : 1
Loc: [/home/jenkins/workspace/Frameworks/kio/kf5-qt5 
SUSEQt5.10/autotests/kdirlistertest.cpp(798)]
  
  After (at home):
   Totals: 32 passed, 0 failed, 0 skipped, 0 blacklisted

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D16349

AFFECTED FILES
  autotests/kdirlistertest.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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
  https://phabricator.kde.org/D16072?vs=43217=43579

REVISION DETAIL
  https://phabricator.kde.org/D16072

AFFECTED FILES
  autotests/kfileplacesviewtest.cpp
  autotests/kfilewidgettest.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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

[Jaime]

Hello,

  About
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.10/429/testReport/junit/(root)/TestSuite/kiowidgets_kdirmodeltest/

  I'm only able to reproduce the crash 1/7 times I run ctest -j3, never if
I run the test alone or run make test.

  I've tried applying what 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

[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

[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

REVISION DETAIL
  https://phabricator.kde.org/D10742

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h
  src/core/kcoredirlister.cpp
  src/core/kcoredirlister_p.h

To: jtamate, #frameworks, dfaure
Cc: bruns, kde-frameworks-devel, mwolff, markg, michaelh, ngraham

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

[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 
manually the tests windows for the test to continue.
  Now it is activated without human intervention.

TEST PLAN
  kfilewidgettest and kfileplaceswidgettest pass without human intervention.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D16072

AFFECTED FILES
  autotests/kfileplacesviewtest.cpp
  autotests/kfilewidgettest.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16020: KFilePlacesModel: fix previous commit to avoid duplicating devices

[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

[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 help because the window/widget needs to 
have the focus.
  
  Is creating a README file and a kwin script to disable stealing prevention 
for kio tests an acceptable solution?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15328

To: jtamate, dfaure, #frameworks
Cc: bruns, anthonyfieroni, broulik, kde-frameworks-devel, michaelh, ngraham

D15328: kfilewidget: convert connect syntax

[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 kfilewidgettest works flawlessly.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15328?vs=41246=43106

REVISION DETAIL
  https://phabricator.kde.org/D15328

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp
  src/filewidgets/kfilewidget.h

To: jtamate, dfaure, #frameworks
Cc: bruns, anthonyfieroni, broulik, kde-frameworks-devel, michaelh, ngraham

D16020: KFilePlacesModel: fix previous commit to avoid duplicating devices

[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 any review request.
  
  With your patch, all the fails from this unit test are gone.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D16020

To: dfaure, jtamate
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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") && (head[3] 
> == 1 || head[3] == 2);
>  }

Shouldn't it be QLatin1String("\x01\xda\x01")?
startsWith has a QLatin1String overloaded method, but will it be used if a 
char* is used as argument or will it use the QString method? I just don't know 
the answer.
Otherwise, +1.

REPOSITORY
  R287 KImageFormats

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15890

To: dfaure, cfeck
Cc: jtamate, kde-frameworks-devel, michaelh, ngraham, bruns

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

[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 code as requested.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10742?vs=35305=42345

REVISION DETAIL
  https://phabricator.kde.org/D10742

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h
  src/core/kcoredirlister.cpp
  src/core/kcoredirlister_p.h

To: jtamate, #frameworks, dfaure
Cc: bruns, kde-frameworks-devel, mwolff, markg, michaelh, ngraham

D15740: Use non deprecated fastInsert in baloo

[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
  https://phabricator.kde.org/D15740

AFFECTED FILES
  src/kioslaves/search/kio_search.cpp

To: jtamate, dfaure, #frameworks, broulik, #baloo, ngraham
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

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

[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

REVISION DETAIL
  https://phabricator.kde.org/D15635

AFFECTED FILES
  src/kioslaves/search/kio_search.cpp
  src/kioslaves/search/kio_search.h

To: jtamate, dfaure, #baloo, #frameworks, ngraham
Cc: ngraham, broulik, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D15740: Use non deprecated fastInsert in baloo

[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.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D15740

AFFECTED FILES
  src/kioslaves/search/kio_search.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

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

[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 for this revision.
jtamate added a comment.


  Copy the methods getUserName and getGroupName from kfile, those are cached, 
should be faster.
  Keep the fastInsert for another revision.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15635?vs=42033=42044

REVISION DETAIL
  https://phabricator.kde.org/D15635

AFFECTED FILES
  src/kioslaves/search/kio_search.cpp
  src/kioslaves/search/kio_search.h

To: jtamate, dfaure, #baloo, #frameworks
Cc: broulik, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15638: force-finish canberra notifications on close()

[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

REVISION DETAIL
  https://phabricator.kde.org/D15638

To: sitter, broulik, jtamate
Cc: kde-frameworks-devel, jtamate, michaelh, ngraham, bruns

D15638: force-finish canberra notifications on close()

[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 know what will happen the second time.
  Perhaps a return after line 161 is needed?

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D15638

To: sitter, broulik
Cc: kde-frameworks-devel, jtamate, michaelh, ngraham, bruns

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

[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.


  White at it, use fastInsert.
  And this also fixes the search in the home directory :-).
  The title in English :-).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15635?vs=42029=42033

REVISION DETAIL
  https://phabricator.kde.org/D15635

AFFECTED FILES
  src/kioslaves/search/kio_search.cpp

To: jtamate, dfaure, #baloo, #frameworks
Cc: broulik, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15623: Finish the notification before removing it from the hash

[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
  https://phabricator.kde.org/D15623

To: jtamate, #frameworks, sitter
Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

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

[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.
  UDS_USER and UDS_GROUP are strings, not numbers, therefore I've got an assert 
(simplified):
  "udsField & KIO::UDSEntry::UDS_NUMBER", "udsentry.cpp", line=line@entry=113)
  
  Use the same methods to get those data as in UDS constructor.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D15635

AFFECTED FILES
  src/kioslaves/search/kio_search.cpp

To: jtamate, dfaure, #baloo, #frameworks
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D15623: Finish the notification before removing it from the hash

[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 
called later by finish, it could contain anything.
  Change the order, call finish with a valid notification pointer before 
removing the notification from the hash.
  
  BUG: 398695

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D15623

AFFECTED FILES
  src/notifybyaudio_canberra.cpp

To: jtamate, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15591: Add Open Document thumbnailer

[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, kossebau, jtamate, fvogt
Cc: leinir, kossebau, jtamate, ngraham

D15591: Add Open Document thumbnailer

[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 calligra installed, which thumbnailer will do 
the work? This on, calligra or both, or depends on the last one being 
installed?  Just for curiosity.

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D15591

To: broulik, #frameworks, #vdg, ngraham, kossebau, jtamate
Cc: kossebau, jtamate, ngraham

D14072: Don't try to restore invalid user places

[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
  https://phabricator.kde.org/D14072

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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
  https://phabricator.kde.org/D13676?vs=37500=41906

REVISION DETAIL
  https://phabricator.kde.org/D13676

AFFECTED FILES
  autotests/kdiroperatortest.cpp
  autotests/kurifiltertest.cpp
  src/filewidgets/kdiroperator.cpp
  src/widgets/kurifilter.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15591: Add Open Document thumbnailer

[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, #frameworks, #vdg
Cc: jtamate, ngraham

D14072: Don't try to restore invalid user places

[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
  https://phabricator.kde.org/D14072

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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 and still return 
text.
  I'm assuming this is for python 3.5 or later.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D15068

To: bruns, #frameworks
Cc: jtamate, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns

D15420: Intialize m_lastPosition

[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
  https://phabricator.kde.org/D15420

AFFECTED FILES
  src/view/kateviewaccessible.h

To: jtamate, #ktexteditor, cullmann, volkov
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D12016: [ktexteditor] much faster positionFromCursor

[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
  https://phabricator.kde.org/D12016

To: jtamate, #kate, cullmann, #frameworks
Cc: kwrite-devel, kde-frameworks-devel, volkov, anthonyfieroni, mwolff, brauch, 
cullmann, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, sars, 
dhaumann

D15420: Intialize m_lastPosition

[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 PLAN
  Run kwrite under valgrind, no message.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D15420

AFFECTED FILES
  src/view/kateviewaccessible.h

To: jtamate, #ktexteditor, cullmann
Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

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

[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 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D14724

To: dfaure, hein, drosca, broulik, davidedmundson
Cc: jtamate, kde-frameworks-devel, #plasma, michaelh, ngraham, bruns

D15408: Don't assert deleting the temporary file

[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 other 
KDE programs to create.
  Remove the asserts. 
  In the very unlikely situation when the preview temporary file has been 
replaced by other temporary file, nothing can be done to avoid to delete the 
file.

TEST PLAN
  With a lot of images in the trash, while dolphin create the previews, empty 
the trash. (It can take a while, around 3000 images, until it asserts).
  Before: a crash in the second assert (Is a file?), because the temporary file 
was already deleted, therefore it is not a file.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15408

AFFECTED FILES
  src/widgets/previewjob.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15328: kfilewidget: convert connect syntax

[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

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15328?vs=41150=41246

REVISION DETAIL
  https://phabricator.kde.org/D15328

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp
  src/filewidgets/kfilewidget.h

To: jtamate, dfaure, #frameworks
Cc: bruns, anthonyfieroni, broulik, kde-frameworks-devel, michaelh, ngraham

D15328: kfilewidget: convert connect syntax

[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: warning: zero as null pointer constant 
[-Wzero-as-null-pointer-constant]

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15328?vs=41148=41150

REVISION DETAIL
  https://phabricator.kde.org/D15328

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp
  src/filewidgets/kfilewidget.h

To: jtamate, dfaure, #frameworks
Cc: anthonyfieroni, broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15328: kfilewidget: convert connect syntax

[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 connect/disconnect work more or less as new/delete?
I mean, if m_connEditTextChanged is reconnected, can't it be disconnected 
safely?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15328

To: jtamate, dfaure, #frameworks
Cc: anthonyfieroni, broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15328: kfilewidget: convert connect syntax

[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 your comment.
m_connEditTextChanged is created in the constructor, line 585.
Afterwards it is disconnected and reconnected in:
setDummyHistoryEntry, removeDummyHistoryEntry and readRecentFiles.

> broulik wrote in kfilewidget.cpp:440
> Shouldn't this be using the `url` from the signal?
> Also, please prefer explicit captures over `&` or `=`

I think it is using the url, the parameter to the lambda and _k_urlEntered(url);

Changed & for this, d can't be used directly.

> broulik wrote in kfilewidget.cpp:473
> Why not change this as well?

Because I didn't knew KActionCollection have it already! Done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15328

To: jtamate, dfaure, #frameworks
Cc: anthonyfieroni, broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15328: kfilewidget: convert connect syntax

[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
  https://phabricator.kde.org/D15328?vs=41145=41148

REVISION DETAIL
  https://phabricator.kde.org/D15328

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp
  src/filewidgets/kfilewidget.h

To: jtamate, dfaure, #frameworks
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15328: kfilewidget: convert connect syntax

[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 open file dialog works.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15328

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp
  src/filewidgets/kfilewidget.h

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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.
  
  
  Yes, without your patch, every time I agree to upload the modified file then 
I have to agree to overwrite the remote file.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15318

To: ngraham, #frameworks, broulik, jtamate
Cc: kde-frameworks-devel, broulik, jtamate, michaelh, ngraham, bruns

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

[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 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15318

To: ngraham, #frameworks, broulik, jtamate
Cc: kde-frameworks-devel, broulik, jtamate, michaelh, ngraham, bruns

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

[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
  https://phabricator.kde.org/D15180?vs=41066=41110

REVISION DETAIL
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: bruns, anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, 
ngraham

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

[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 elements, looks like the compiler agrees:
kioexecd.cpp:138:36: error: no matching function for call to ‘QMap::erase(QMap::const_iterator&)’

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15180

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: bruns, anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, 
ngraham

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

[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
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: bruns, anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, 
ngraham

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

[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 modifications of the temporary files' has not been accepted. 
Continue anyway? [y/N]
  
  > Even better use the std::remove_if/std::erase pattern, which avoids O(n^2) 
complexity during erase.
  > Or in this specific case, std::partition/{iterate over removed}/std::erase
  
  I assume QMap is like a std::map. I've seen in this cppreference page 
 that these algorithms 
cannot be used with associative containers such as std::set and std::map.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15180

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: bruns, anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, 
ngraham

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

[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
  https://phabricator.kde.org/D15180?vs=41030=41035

REVISION DETAIL
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

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

[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
  - Security mistakes
  - Misunderstanding the API.
  
  Definitely, I need more vacation.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15180?vs=41026=41030

REVISION DETAIL
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

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

[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 SINCE LAST UPDATE
  https://phabricator.kde.org/D15180?vs=40961=41026

REVISION DETAIL
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

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

[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")` ?
> 
> At the very least we need to check whether this folder starts with 
> `QStandardPaths::writableLocation(QStandardPaths::CacheLocation) + 
> QStringLiteral("/krun")` (the path used by `kioexec`).

There is a slightly problem:  QStandardPaths::CacheLocation is application 
dependent, and their values doesn't match here:
kioexec: /home/jtorres/.cache/kioexec/
kioexecd: /home/jtorres/.cache/kiod5/
Can we assume that replacing kiod5 by kioexec will always work?

We could use QStandardPaths::GenericCacheLocation instead, but this is not 
guaranteed to be non empty.

Or another solution: keep it as it was (delete only the file and the directory 
if it is possible).

> anthonyfieroni wrote in kioexecd.cpp:85-88
> Now it's not needed 'remove' will do the work

You're right. In this case it isn't possible to be notified of a file creation 
unless it has been deleted first.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15180

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

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

[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
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h
  src/kioexec/main.cpp

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

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

[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
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h
  src/kioexec/main.cpp

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

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

[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
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h
  src/kioexec/main.cpp

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

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

[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
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15180?vs=40888=40908

REVISION DETAIL
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h
  src/kioexec/main.cpp

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

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

[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. So stop watching should not depend on process return 
code, also same command should not stop container to shrink, that's my opinion, 
but i can miss something.
  
  
  I desist from this path, it has a big flaw: What if the tabbed application is 
already running? All the temporary files will be deleted immediately. :-(
  I'll try this other path: allowing the application a generous amount of time 
(30s) to recreate the deleted file, otherwise deleting the temporary directory. 
Unfortunately, the temporary directories will not shrink unless the app deletes 
the temporary file.
  I guess the only complete solution is to implement use fuse (where available) 
.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15180

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: anthonyfieroni, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

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

[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
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h
  src/kioexec/main.cpp

To: jtamate, #frameworks, broulik, ngraham, dfaure, elvisangelaccio
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

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

[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

[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

kioexec creates a folder per temp file.
kioexec doesn't remove recursively, therefore the temporary directory will not 
be deleted if the program creates backup files, like a.txt~

> broulik wrote in kioexecd.cpp:73
> I see. Previously it would just remove the watcher when the temp file is 
> removed. Not sure how to fix it now, maybe we need an `unwatch` dbus call.
> Imho we shouldn't change the behavior here too much, ie. "randomly" watching 
> the parent dir instead of the actual file passed in as argument but instead 
> introduce a `watchDirectory` dbus call that exhibits the new behavior? Not 
> sure.

The watch will be left as it was, introducing the create case and removing the 
deleted case.

I've implemented an unwatch dbus call.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15180

To: jtamate, #frameworks, broulik, ngraham, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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 still there.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15180?vs=40737=40779

REVISION DETAIL
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h
  src/kioexec/main.cpp

To: jtamate, #frameworks, broulik, ngraham, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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 dir

No, only at the destructor, unless there is a way to know when the program that 
uses the file has been closed, I guess keeping the file during all the session 
is the same behavior as before.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15180

To: jtamate, #frameworks, broulik, ngraham, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

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

[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, the file is copied to 
an user temporary folder.
  Watch any creation or modification in that temporary folder. In that way, 
even with programs that delete the old file and create a new one when saving 
the file, the user is asked if he wants to upload the modified file.
  
  Delete recursively the temporary directories and any file they could have in 
the destructor.
  
  BUG: 397742

TEST PLAN
  Tested logging out and in several times and opening remote files with xed and 
libreoffice.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15180

AFFECTED FILES
  src/kioexec/kioexecd.cpp
  src/kioexec/kioexecd.h

To: jtamate, #frameworks, broulik, ngraham, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D11604: kdirlistertest doesn't fail at random

[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
  https://phabricator.kde.org/D11604

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h

To: jtamate, #frameworks, dfaure
Cc: kde-frameworks-devel, apol, michaelh, ngraham, bruns

D11604: kdirlistertest doesn't fail at random

[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);
QCOMPARE(m_dirLister.spyClearQUrl.count(), 0);
  
  Otherwise the test fails for me.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11604?vs=39990=39991

REVISION DETAIL
  https://phabricator.kde.org/D11604

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h

To: jtamate, #frameworks, dfaure
Cc: kde-frameworks-devel, apol, michaelh, ngraham, bruns

D11604: kdirlistertest doesn't fail at random

[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
  https://phabricator.kde.org/D11604

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h

To: jtamate, #frameworks, dfaure
Cc: kde-frameworks-devel, apol, michaelh, ngraham, bruns

D13813: Make this test work again with new uds implementation

[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

REVISION DETAIL
  https://phabricator.kde.org/D13813

To: jtamate, #dolphin, #frameworks, elvisangelaccio
Cc: bcooksley, bshah, nalvarez, cfeck, dfaure, aacid, ngraham, bruns, 
elvisangelaccio, kfm-devel, spoorun, navarromorales, firef, andrebarros, 
emmanuelp

D11604: kdirlistertest doesn't fail at random

[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
  https://phabricator.kde.org/D11604?vs=39801=39836

REVISION DETAIL
  https://phabricator.kde.org/D11604

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h

To: jtamate, #frameworks, dfaure
Cc: kde-frameworks-devel, apol, michaelh, ngraham, bruns

D11604: kdirlistertest doesn't fail at random

[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 connections to the new syntax.
  Have run it several times and it hasn't failed here.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11604?vs=33623=39801

REVISION DETAIL
  https://phabricator.kde.org/D11604

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h

To: jtamate, #frameworks, dfaure
Cc: kde-frameworks-devel, apol, michaelh, ngraham, bruns

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

[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
  src/tools/balooshow/main.cpp

To: jtamate, #frameworks, pino
Cc: pino, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

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

[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 > 0);
  so don't even try to call it with a 0 id.

TEST PLAN
  Executing balooshow -i /home/user/* when baloo is creating the index after 
deleting .local/share/baloo
  before: crash (assert)
  after: balooshow ends normally.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D14859

AFFECTED FILES
  src/tools/balooshow/main.cpp

To: jtamate, #frameworks
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D13813: Make this test work again with new uds implementation

[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 5.43.
  
  But it could land as it is and be ready for just change insert by fastInsert 
when it depends on that kio version.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D13813

To: jtamate, #dolphin, #frameworks, elvisangelaccio
Cc: dfaure, aacid, ngraham, bruns, elvisangelaccio, kfm-devel, spoorun, 
navarromorales, firef, andrebarros, emmanuelp

D12945: kcoredirlister lstItems benchmark

[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
  https://phabricator.kde.org/D12945?vs=38134=38612

REVISION DETAIL
  https://phabricator.kde.org/D12945

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/kcoredirlister_benchmark.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D12945: kcoredirlister lstItems benchmark

[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

[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 (I've read your comments this 
morning), clementine appeared suddenly playing a song.

REPOSITORY
  R271 KDBusAddons

REVISION DETAIL
  https://phabricator.kde.org/D14302

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns

D14302: Don't block forever in ensureKdeinitRunning

[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 DETAIL
  https://phabricator.kde.org/D14302

AFFECTED FILES
  src/kdeinitinterface.cpp

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns

D14302: Don't block forever in ensureKdeinitRunning

[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 UPDATE
  https://phabricator.kde.org/D14302?vs=38387=38403

REVISION DETAIL
  https://phabricator.kde.org/D14302

AFFECTED FILES
  src/kdeinitinterface.cpp

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns

D14302: Don't block forever in ensureKdeinitRunning

[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 understood, again, the 
code.
  
  Added your description as comments.

REPOSITORY
  R271 KDBusAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14302?vs=38372=38387

REVISION DETAIL
  https://phabricator.kde.org/D14302

AFFECTED FILES
  src/kdeinitinterface.cpp

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns

D14302: Don't block forever in ensureKdeinitRunning

[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 counter intuitive backtrace, 
take a look at the dissasembler.
  Thanks everybody, specially to @thiago.

REPOSITORY
  R271 KDBusAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14302?vs=38264=38372

REVISION DETAIL
  https://phabricator.kde.org/D14302

AFFECTED FILES
  src/kdeinitinterface.cpp

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns