dmitrio marked an inline comment as done.
dmitrio added a comment.

  In D10663#209708 <https://phabricator.kde.org/D10663#209708>, @meven wrote:
  
  > Add this to your commit message
  >  FIXED-IN: 5.44
  
  
  Thank you, added it to the description.
  
  In D10663#209711 <https://phabricator.kde.org/D10663#209711>, @anthonyfieroni 
wrote:
  
  > It looks D10635 <https://phabricator.kde.org/D10635> is duplicate.
  
  
  In fact, this revision is a duplicate of D10635 
<https://phabricator.kde.org/D10635>. I am a bit surprised why this one has a 
smaller number :)
  
  In D10663#209720 <https://phabricator.kde.org/D10663#209720>, @meven wrote:
  
  > It was suggested on IRC to hide this behavior behind a flag such as the 
jobFlag enum, so that it can be opt-in/opt-out in applications.
  
  
  I agree with this proposal, but I can suggest that the added flag should 
rather turn off the new behavior, so the default behavior will be to do this 
cleanup on job cancel.
  However, if more people think that it should not be turned on by default, it 
would be easy to change this diff to achieve an opposite behavior.
  
  In D10663#209720 <https://phabricator.kde.org/D10663#209720>, @meven wrote:
  
  > We could consider changing the job state during the clean up, something like
  >
  >   d->state == STATE_CLEANING_INCOMPLETE_FILE
  >
  
  
  For now, I see no point in this change, since, to avoid breaking a logic of 
doKill() method, the deletion job is currently launched not as subjob, but as a 
separate job. So, after doKill() execution the CopyJob does nothing anymore (as 
it is supposed to be) and has no need in tracking its state. Feel free to 
correct me if I am wrong.

INLINE COMMENTS

> meven wrote in copyjob.cpp:559
> I believe this is because files are copied in sequence, so at any given time 
> only the first file is being copied.
> 
> Maybe you could use m_currentSrcURL to avoid accessing the iterator.

You are right, updated a diff.

On the question on deleting first file only, yes, we need to delete only the 
file that was being copied before the user canceled the job. Perhaps using 
m_currentDestURL would make this intention more clear.

REPOSITORY
  R241 KIO

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

To: dmitrio, #frameworks, dfaure
Cc: anthonyfieroni, meven, #frameworks, michaelh

Reply via email to