From 1f1414026024f6055c384a613e3425393db7bb51 Mon Sep 17 00:00:00 2001
From: Nils <>
Date: Sun, 20 Sep 2020 19:11:49 +0200
Subject: [PATCH] Fix unlink block by reusing existing code

---
 engine/api.py              | 18 +++++++++++++++---
 engine/block.py            | 11 +++++++----
 engine/track.py            |  1 +
 template/engine/history.py |  6 +++---
 4 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/engine/api.py b/engine/api.py
index 07253cc..ba13f05 100644
--- a/engine/api.py
+++ b/engine/api.py
@@ -897,11 +897,23 @@ def unlinkCurrentBlock():
 
 def unlinkBlock(blockId):
     track, block = session.data.blockById(blockId)
-    newData = block.getUnlinkedData()
-    assert newData
-    _setBlockData(block, newData) #handles undo and callbacks
+    #if len(block.linkedContentBlocks) == 1: return  #This will not work because block is still the old one. unlink replaces with a standalone copy but we keep the original for undo.
 
+    with session.history.sequence("unlink block"):
+        duplicateBlock(blockId)
+        deleteBlock(blockId) #original one
+    callbacks._historyChanged()
+
+    #Does not work properly:
+    #newData = block.getUnlinkedData()
+    #assert newData
+    #_setBlockData(block, newData) #handles undo and callbacks
+
+#deprecated
 def _setBlockData(block, newData):
+    """DEPRECATED. This did not work. The linked blocks were not reduced.
+    At point of the bug reports, instead of fixing this, we used
+    duplicate block and delete which already provides all functionality"""
     session.history.register(lambda bl=block, old=block.data: _setBlockData(bl, old), descriptionString = "set block data")
     block.data = newData
     #no callbacks needed.
diff --git a/engine/block.py b/engine/block.py
index 95a3fff..92ad25e 100644
--- a/engine/block.py
+++ b/engine/block.py
@@ -153,9 +153,9 @@ class Block(object):
         for item in self.data:
             copyItem = item.copy()
             new.data.append(copyItem)
-            
+
             assert not copyItem.parentBlocks #parentBlock was empty until now
-            copyItem.parentBlocks.add(new)  
+            copyItem.parentBlocks.add(new)
             if self in copyItem.parentBlocks: #TODO: investigate
                 copyItem.parentBlocks.remove(self)
 
@@ -167,7 +167,10 @@ class Block(object):
         return new
 
     def getUnlinkedData(self):
-        """Set and handled for undo/redo by the api"""
+        """
+        Returns a new list with copies of items.
+        Set and handled for undo/redo by the api.
+        """
         newData = []
         newParentBlocks = WeakSet()
         newParentBlocks.add(self)
@@ -304,7 +307,7 @@ class Block(object):
     def insert(self, item):
         self.data.insert(self.localCursorIndex, item) #we do not need to check if appending or not. list.insert appends if the index is higher then len()
         #self.localCursorIndex += 1 #we don't need to go right here because track.insert() is calling its own right() directly after insert, which triggers block.right()
-        item.parentBlocks.add(self)        
+        item.parentBlocks.add(self)
 
     def delete(self):
         """The commented out is the immediate garbage collector which
diff --git a/engine/track.py b/engine/track.py
index cca7492..46205a4 100644
--- a/engine/track.py
+++ b/engine/track.py
@@ -436,6 +436,7 @@ class Track(object):
         return new
 
     def appendExistingBlock(self, block):
+        assert type(block) is Block, block
         block.parentTrack = self
         self.blocks.append(block)
 
diff --git a/template/engine/history.py b/template/engine/history.py
index f50edff..76a4135 100644
--- a/template/engine/history.py
+++ b/template/engine/history.py
@@ -69,7 +69,7 @@ class History(object):
     def sequence(self, descriptionString):
         """Convenience function. All registrations from now on will be undone at once.
         call stopSequence when done.
-        This is not meant for high level scripts and not for user-interaction.
+        This is meant for high level scripts and not for user-interaction.
 
         This context can also be used to force a custom name for the undo description, even if it is
         only one step.
@@ -94,11 +94,11 @@ class History(object):
 
 
     def register(self, registeredUndoFunction, descriptionString):
-        """Register a single undo function but use the same syntax as many functions"""        
+        """Register a single undo function but use the same syntax as many functions"""
         self._register(descriptionString, [registeredUndoFunction,])
 
     def _register(self, descriptionString, listOfFunctions):
-        assert type(listOfFunctions) is list, (type(listOfFunctions), listOfFunctions)        
+        assert type(listOfFunctions) is list, (type(listOfFunctions), listOfFunctions)
         if self.doNotRegisterRightNow:
             self.redoStack.append((descriptionString, listOfFunctions))
         else: