Review Request 128527: FileUndoManager: Undoing symlink creation

[Anthony Fieroni]

> On Юли 31, 2016, 12:47 след обяд, 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.
> 
> Chinmoy Ranjan Pradhan wrote:
>     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.

Looks strange to me, what will be happend if it has type (BasicOperation::Link) and commandType (FileUndoManager::Link)? If i'm not wrong, element must be appended on Link operations.


- Anthony


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


On Юли 31, 2016, 12:28 след обяд, Chinmoy Ranjan Pradhan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128527/
> -----------------------------------------------------------
> 
> (Updated Юли 31, 2016, 12:28 след обяд)
> 
> 
> 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/20160801/c9ae0efc/attachment-0001.html>