[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    KDE/kdebase/libkonq
From:       David Faure <faure () kde ! org>
Date:       2007-01-20 10:57:43
Message-ID: 1169290663.733187.12461.nullmailer () svn ! kde ! org
[Download RAW message or body]

SVN commit 625552 by dfaure:

Cleanups; unit test undoing a directory move.


 M  +69 -75    konq_undo.cc  
 M  +0 -2      konq_undo.h  
 M  +21 -0     tests/konqundomanagertest.cpp  
 M  +2 -0      tests/konqundomanagertest.h  


--- trunk/KDE/kdebase/libkonq/konq_undo.cc #625551:625552
@@ -63,6 +63,12 @@
  *
  */
 
+enum UndoState { MAKINGDIRS = 0, MOVINGFILES, REMOVINGDIRS, REMOVINGFILES };
+static const char* undoStateToString( UndoState state ) {
+    static const char* s_undoStateToString[] = { "MAKINGDIRS", "MOVINGFILES", \
"REMOVINGDIRS", "REMOVINGFILES" }; +    return s_undoStateToString[state];
+}
+
 class KonqUndoJob : public KIO::Job
 {
 public:
@@ -238,99 +244,87 @@
 
 bool KonqUndoManager::undoAvailable() const
 {
-  return ( d->m_commands.count() > 0 ) && !d->m_lock;
+    return ( d->m_commands.count() > 0 ) && !d->m_lock;
 }
 
 QString KonqUndoManager::undoText() const
 {
-  if ( d->m_commands.isEmpty() )
-    return i18n( "Und&o" );
+    if ( d->m_commands.isEmpty() )
+        return i18n( "Und&o" );
 
-  KonqUndoManager::CommandType t = d->m_commands.top().m_type;
-  if ( t == KonqUndoManager::COPY )
-    return i18n( "Und&o: Copy" );
-  else if ( t == KonqUndoManager::LINK )
-    return i18n( "Und&o: Link" );
-  else if ( t == KonqUndoManager::MOVE )
-    return i18n( "Und&o: Move" );
-  // distinguish renaming from moving - #61442
-  else if ( t == KonqUndoManager::RENAME )
-    return i18n( "Und&o: Rename" );
-  else if ( t == KonqUndoManager::TRASH )
-    return i18n( "Und&o: Trash" );
-  else if ( t == KonqUndoManager::MKDIR )
-    return i18n( "Und&o: Create Folder" );
-  else
-    assert( false );
-  /* NOTREACHED */
-  return QString();
+    KonqUndoManager::CommandType t = d->m_commands.top().m_type;
+    if ( t == KonqUndoManager::COPY )
+        return i18n( "Und&o: Copy" );
+    else if ( t == KonqUndoManager::LINK )
+        return i18n( "Und&o: Link" );
+    else if ( t == KonqUndoManager::MOVE )
+        return i18n( "Und&o: Move" );
+    else if ( t == KonqUndoManager::RENAME )
+        return i18n( "Und&o: Rename" );
+    else if ( t == KonqUndoManager::TRASH )
+        return i18n( "Und&o: Trash" );
+    else if ( t == KonqUndoManager::MKDIR )
+        return i18n( "Und&o: Create Folder" );
+    else
+        assert( false );
+    /* NOTREACHED */
+    return QString();
 }
 
 void KonqUndoManager::undo()
 {
-  KonqCommand cmd = d->m_commands.top();
-  broadcastPop();
-  broadcastLock();
+    // Make a copy of the command to undo before broadcastPop() pops it.
+    KonqCommand cmd = d->m_commands.top();
+    broadcastPop();
+    broadcastLock();
 
-  assert( cmd.m_valid );
+    assert( cmd.m_valid );
 
-  d->m_current = cmd;
-  d->m_dirCleanupStack.clear();
-  d->m_dirStack.clear();
-  d->m_dirsToUpdate.clear();
+    d->m_current = cmd;
+    d->m_dirCleanupStack.clear();
+    d->m_dirStack.clear();
+    d->m_dirsToUpdate.clear();
 
-  d->m_undoState = MOVINGFILES;
-  kDebug(1203) << "KonqUndoManager::undo MOVINGFILES" << endl;
+    d->m_undoState = MOVINGFILES;
+    // Let's have a look at the basic operations we need to undo.
+    // While we're at it, collect all links that should be deleted.
 
-  KonqBasicOperation::Stack& opStack = d->m_current.m_opStack;
-  assert( !opStack.isEmpty() );
+    KonqBasicOperation::Stack& opStack = d->m_current.m_opStack;
+    assert( !opStack.isEmpty() );
 
-  QStack<KonqBasicOperation>::Iterator it = opStack.begin();
-  while ( it != opStack.end() )
-  {
-    bool removeBasicOperation = false;
-    if ( (*it).m_directory && !(*it).m_renamed )
+    QStack<KonqBasicOperation>::Iterator it = opStack.begin();
+    while ( it != opStack.end() ) // don't cache end() here, erase modifies it
     {
-      d->m_dirStack.push( (*it).m_src );
-      d->m_dirCleanupStack.prepend( (*it).m_dst );
-      removeBasicOperation = true;
-      d->m_undoState = MAKINGDIRS;
-      kDebug(1203) << "KonqUndoManager::undo MAKINGDIRS" << endl;
-    }
-    else if ( (*it).m_link )
-    {
-      if ( !d->m_fileCleanupStack.contains( (*it).m_dst ) )
-        d->m_fileCleanupStack.prepend( (*it).m_dst );
+        bool removeBasicOperation = false;
+        if ( (*it).m_directory && !(*it).m_renamed )
+        {
+            // If any directory has to be created/deleted, we'll start with that
+            d->m_undoState = MAKINGDIRS;
+            // Collect all the dirs that have to be created in case of a move undo.
+            if ( d->m_current.isMoveCommand() )
+                d->m_dirStack.push( (*it).m_src );
+            // Collect all dirs that have to be deleted
+            // from the destination in both cases (copy and move).
+            d->m_dirCleanupStack.prepend( (*it).m_dst );
+            removeBasicOperation = true;
+        }
+        else if ( (*it).m_link )
+        {
+            if ( !d->m_fileCleanupStack.contains( (*it).m_dst ) )
+                d->m_fileCleanupStack.prepend( (*it).m_dst );
 
-      removeBasicOperation = !d->m_current.isMoveCommand();
+            removeBasicOperation = !d->m_current.isMoveCommand();
+        }
+
+        if ( removeBasicOperation )
+            it = opStack.erase( it );
+        else
+            ++it;
     }
 
-    if ( removeBasicOperation )
-      it = opStack.erase( it );
-    else
-      ++it;
-  }
-
-  /* this shouldn't be necessary at all:
-   * 1) the source list may contain files, we don't want to
-   *    create those as... directories
-   * 2) all directories that need creation should already be in the
-   *    directory stack
-  if ( d->m_undoState == MAKINGDIRS )
-  {
-    KUrl::List::ConstIterator it = d->m_current.m_src.begin();
-    KUrl::List::ConstIterator end = d->m_current.m_src.end();
-    for (; it != end; ++it )
-      if ( !d->m_dirStack.contains( *it) )
-        d->m_dirStack.push( *it );
-  }
-  */
-
-  if ( !d->m_current.isMoveCommand() )
-    d->m_dirStack.clear();
-
-  d->m_undoJob = new KonqUndoJob;
-  undoStep();
+    kDebug(1203) << "KonqUndoManager::undo starting with " << \
undoStateToString(d->m_undoState) << endl; +    d->m_undoJob = new KonqUndoJob;
+    undoStep();
 }
 
 void KonqUndoManager::stopUndo( bool step )
--- trunk/KDE/kdebase/libkonq/konq_undo.h #625551:625552
@@ -99,8 +99,6 @@
 private:
   KonqUndoManager();
 
-  enum UndoState { MAKINGDIRS, MOVINGFILES, REMOVINGDIRS, REMOVINGFILES };
-
   friend class KonqUndoJob;
   /// called by KonqUndoJob
   void stopUndo( bool step );
--- trunk/KDE/kdebase/libkonq/tests/konqundomanagertest.cpp #625551:625552
@@ -258,4 +258,25 @@
     QVERIFY( !QFile::exists( destSubDir() ) );
 }
 
+void KonqUndomanagerTest::testMoveDirectory()
+{
+    const QString destdir = destDir();
+    KUrl::List lst; lst << srcSubDir();
+    const KUrl d( destdir );
+    KIO::CopyJob* job = KIO::move( lst, d, 0 );
+    job->setUiDelegate( 0 );
+    KonqUndoManager::self()->recordJob( KonqUndoManager::MOVE, lst, d, job );
+
+    bool ok = KIO::NetAccess::synchronousRun( job, 0 );
+    QVERIFY( ok );
+
+    QVERIFY( !QFile::exists( srcSubDir() ) );
+    checkTestDirectory( destSubDir() );
+
+    doUndo();
+
+    checkTestDirectory( srcSubDir() );
+    QVERIFY( !QFile::exists( destSubDir() ) );
+}
+
 // TODO: add test for undoing after a partial move \
                (http://bugs.kde.org/show_bug.cgi?id=91579)
--- trunk/KDE/kdebase/libkonq/tests/konqundomanagertest.h #625551:625552
@@ -32,6 +32,8 @@
     void testMoveFiles();
     //void testCopyFilesOverwrite();
     void testCopyDirectory();
+    void testMoveDirectory();
+    // TODO testRenameFile/testRenameDir
     // TODO testTrashFiles
 
 private:


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic