Review Request 128527: FileUndoManager: Undoing symlink creation

[Chinmoy Ranjan Pradhan]

> On July 31, 2016, 9:47 a.m., David Faure wrote:
> > src/widgets/fileundomanager.cpp, line 421
> > <https://git.reviewboard.kde.org/r/128527/diff/2/?file=472624#file472624line421>
> >
> >     Why the isEmpty() check?
> 
> Chinmoy Ranjan Pradhan wrote:
>     For KIO::SimpleJob opStack is empty due to which m_fileCleanupStack is empty. So isEmpty() check is required.
> 
> David Faure wrote:
>     You're saying "the stack is empty", then why is it needed to check that it's empty?
>     
>     Is this to avoid doing the newly-added append() when doing a full-directory CopyJob ?
>     But then it means, if the first file being copied, in that directory, is a symlink, we'll go into this if (because the stack is still empty) and append the link (again), no? (there's no unittest for that...).
>     
>     If I'm right then maybe we need a new enum value, SingleLink or something.

Yes, we end up here in case CopyJob::link or ::linkAs is used. To avoid an append() i think this check  is necessary.  
But (i guess) if CopyJob is used then stack will never be empty. So IMO this if statement is only true when the job is SimpleJob and tyype is symlink.


- Chinmoy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128527/#review97941
-----------------------------------------------------------


On July 31, 2016, 9:28 a.m., Chinmoy Ranjan Pradhan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128527/
> -----------------------------------------------------------
> 
> (Updated July 31, 2016, 9:28 a.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez, David Faure, Eike Hein, and Wolfgang Bauer.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> This patch adds the support for undoing creation of new symlinks.
> 
> 
> Diffs
> -----
> 
>   autotests/fileundomanagertest.h c663f92 
>   autotests/fileundomanagertest.cpp 761cc76 
>   src/filewidgets/knewfilemenu.cpp bb6fc04 
>   src/widgets/fileundomanager.cpp ed5edb0 
> 
> Diff: https://git.reviewboard.kde.org/r/128527/diff/
> 
> 
> Testing
> -------
> 
> build
> 
> 
> Thanks,
> 
> Chinmoy Ranjan Pradhan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160731/fdf98f18/attachment.html>