D7841: kioexec: Do not start to watch a temporary copy until after the copy is complete

2017-09-15 Thread Jonathan Marten
marten created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  As described in bug https://bugs.kde.org/show_bug.cgi?id=384500, there 
appears to be a problem when the receiving application of a file needs a 
temporary copy to be made (because of %F/%f in its desktop file).  The kded 
file watching module is told to watch the file too early, before the ioslave 
has even started to copy it.  This means that when the copy is complete it will 
receive a dirty signal (on file creation) and the user will immediately be 
prompted to re-upload.
  
  This change moves the file watch operation to after the file copy job is 
complete.  At this point the file is in a stable state and hence the dirty 
signal and the prompt will not happen, unless the file really is subsequently 
modified.

TEST PLAN
  Built and installed kio with this change, checked remote file opening as per 
the bug.  Verified that the kded module is not told to watch the file until 
after the copy is complete, and that there is no re-upload prompt unless the 
file is actually modified.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/kioexec/main.cpp

To: marten, #frameworks
Cc: elvisangelaccio, dfaure


D7841: kioexec: Do not start to watch a temporary copy until after the copy is complete

2017-09-24 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  Makes sense.

INLINE COMMENTS

> main.cpp:151
> +for (; it != fileList.end(); ++it) {
> +if (it->path == path) {
> +break;

(pre-existing) std::find_if with a lambda would be shorter and (imho) more 
readable, but feel free to ignore this if you're not at ease with that

> main.cpp:166
> +const QString dest = copyJob->srcUrl().toString();
> +qDebug() << "path" << path;
> +qDebug() << "dest" << dest;

clean up the debug output please

REPOSITORY
  R241 KIO

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

To: marten, #frameworks, dfaure
Cc: elvisangelaccio, dfaure


D7841: kioexec: Do not start to watch a temporary copy until after the copy is complete

2017-09-24 Thread Jonathan Marten
marten marked 2 inline comments as done.

REPOSITORY
  R241 KIO

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

To: marten, #frameworks, dfaure
Cc: elvisangelaccio, dfaure


D7841: kioexec: Do not start to watch a temporary copy until after the copy is complete

2017-09-24 Thread Jonathan Marten
marten updated this revision to Diff 19864.
marten added a comment.


  Updated in accordance with review comments.  Go easy - it's the first lambda 
that I've submitted...

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7841?vs=19565&id=19864

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

AFFECTED FILES
  src/kioexec/main.cpp

To: marten, #frameworks, dfaure
Cc: elvisangelaccio, dfaure


D7841: kioexec: Do not start to watch a temporary copy until after the copy is complete

2017-09-24 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.


  Perfect, thanks.

REPOSITORY
  R241 KIO

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

To: marten, #frameworks, dfaure
Cc: elvisangelaccio, dfaure


D7841: kioexec: Do not start to watch a temporary copy until after the copy is complete

2017-09-25 Thread Jonathan Marten
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:c76c1486ec79: kioexec: Watch the file when it has 
finished copying (authored by marten).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7841?vs=19864&id=19885

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

AFFECTED FILES
  src/kioexec/main.cpp

To: marten, #frameworks, dfaure
Cc: elvisangelaccio, dfaure