Merge pull request #1152 from rachel-fenichel/feature/delete_procedure

Feature/delete procedure

blocks_vertical/procedures.js

@@ -148,6 +148,15 @@ Blockly.Blocks['procedures_callnoreturn_internal'] = {
     this.argumentDefaults_ = [];
     this.warp_ = false;
   },
+  /**
+   * Returns the name of the procedure this block calls, or the empty string if
+   * it has not yet been set.
+   * @return {string} Procedure name.
+   * @this Blockly.Block
+   */
+  getProcCode: function() {
+    return this.procCode_;
+  },
   /**
    * Create XML to represent the (non-editable) name and arguments.
    * @return {!Element} XML storage element.

blocks_vertical/vertical_extensions.js

@@ -160,11 +160,25 @@ Blockly.ScratchBlocks.VerticalExtensions.PROCEDURE_DEF_CONTEXTMENU = {
     // Add the edit option at the end.
     menuOptions.push(Blockly.Procedures.makeEditOption(this));
 
-    // Find the delete option and update its callback to be specific to functions.
+    // Find the delete option and update its callback to be specific to
+    // functions.
     for (var i = 0, option; option = menuOptions[i]; i++) {
       if (option.text == Blockly.Msg.DELETE_BLOCK) {
+        var input = this.getInput('custom_block');
+        // this is the root block, not the shadow block.
+        if (input && input.connection && input.connection.targetBlock()) {
+          var procCode = input.connection.targetBlock().getProcCode();
+        } else {
+          return;
+        }
+        var rootBlock = this;
         option.callback = function() {
-          alert('TODO(#1130): implement function deletion');
+          var didDelete = Blockly.Procedures.deleteProcedureDefCallback(
+              procCode, rootBlock);
+          if (!didDelete) {
+            // TODO:(#1151)
+            alert('To delete a block definition, first remove all uses of the block');
+          }
         };
       }
     }

core/procedures.js

@@ -276,7 +276,7 @@ Blockly.Procedures.getCallers = function(name, ws, definitionRoot,
   var callers = [];
   for (var i = 0; i < allBlocks.length; i++) {
     var block = allBlocks[i];
-    if (block.type == 'procedure_callnoreturn') {
+    if (block.type == 'procedures_callnoreturn') {
       var procCode = block.getProcCode();
       if (procCode && procCode == name) {
         callers.push(block);
@@ -412,3 +412,25 @@ Blockly.Procedures.makeShowDefinitionOption = function(block) {
   };
   return option;
 };
+
+/**
+ * Callback to try to delete a custom block definitions.
+ * @param {string} procCode The identifier of the procedure to delete.
+ * @param {!Blockly.Block} definitionRoot The root block of the stack that
+ *     defines the custom procedure.
+ * @return {boolean} True if the custom procedure was deleted, false otherwise.
+ * @package
+ */
+Blockly.Procedures.deleteProcedureDefCallback = function(procCode,
+    definitionRoot) {
+  var callers = Blockly.Procedures.getCallers(procCode,
+      definitionRoot.workspace, definitionRoot, false /* allowRecursive */);
+  if (callers.length > 0) {
+    return false;
+  }
+  // Delete the whole stack.
+  Blockly.Events.setGroup(true);
+  definitionRoot.dispose();
+  Blockly.Events.setGroup(false);
+  return true;
+};

tests/jsunit/procedure_test.js

@@ -26,7 +26,7 @@ var workspace;
 //var mockControl_;
 
 function procedureTest_setUp() {
-  Blockly.Blocks['procedure_callnoreturn'] = {
+  Blockly.Blocks['procedures_callnoreturn'] = {
     init: function() {
       this.procCode_ = '';
       this.setPreviousStatement(true);
@@ -64,7 +64,7 @@ function procedureTest_setUp() {
 }
 
 function procedureTest_tearDown() {
-  delete Blockly.Blocks['procedure_callnoreturn'];
+  delete Blockly.Blocks['procedures_callnoreturn'];
   delete Blockly.Blocks['foo'];
   delete Blockly.Blocks['loop'];
   //mockControl_.$tearDown();
@@ -74,7 +74,7 @@ function procedureTest_tearDown() {
 function test_findCallers_simple_oneCaller() {
   var xml = '<xml xmlns="http://www.w3.org/1999/xhtml">' +
     '<variables></variables>' +
-    '<block type="procedure_callnoreturn" id="test_1" x="301" y="516">' +
+    '<block type="procedures_callnoreturn" id="test_1" x="301" y="516">' +
     '</block>' +
   '</xml>';
   procedureTest_setUp();
@@ -94,7 +94,7 @@ function test_findCallers_simple_oneCaller() {
 function test_findCallers_noRecursion() {
   var xml = '<xml xmlns="http://www.w3.org/1999/xhtml">' +
     '<variables></variables>' +
-    '<block type="procedure_callnoreturn" id="test_1" x="301" y="516">' +
+    '<block type="procedures_callnoreturn" id="test_1" x="301" y="516">' +
     '</block>' +
   '</xml>';
   procedureTest_setUp();
@@ -117,7 +117,7 @@ function test_findCallers_noRecursion() {
 function test_findCallers_allowRecursion() {
   var xml = '<xml xmlns="http://www.w3.org/1999/xhtml">' +
     '<variables></variables>' +
-    '<block type="procedure_callnoreturn" id="test_1" x="301" y="516">' +
+    '<block type="procedures_callnoreturn" id="test_1" x="301" y="516">' +
     '</block>' +
   '</xml>';
   procedureTest_setUp();
@@ -150,7 +150,7 @@ function test_findCallers_simple_noCallers() {
     var callers = Blockly.Procedures.getCallers('test_procedure', workspace,
         {id: ''}, false);
 
-    // There weren't even blocks of type procedure_callnoreturn.
+    // There weren't even blocks of type procedures_callnoreturn.
     assertEquals(0, callers.length);
   }
   finally {
@@ -161,7 +161,7 @@ function test_findCallers_simple_noCallers() {
 function test_findCallers_wrongProcCode() {
   var xml = '<xml xmlns="http://www.w3.org/1999/xhtml">' +
     '<variables></variables>' +
-    '<block type="procedure_callnoreturn" id="test_1" x="301" y="516">' +
+    '<block type="procedures_callnoreturn" id="test_1" x="301" y="516">' +
     '</block>' +
   '</xml>';
   procedureTest_setUp();
@@ -185,7 +185,7 @@ function test_findCallers_onStatementInput() {
       '<statement name="SUBSTACK">' +
         '<block type="foo" id="test_2">' +
           '<next>' +
-            '<block type="procedure_callnoreturn" id="test_3"></block>' +
+            '<block type="procedures_callnoreturn" id="test_3"></block>' +
           '</next></block>' +
       '</statement>' +
     '</block>' +
@@ -209,7 +209,7 @@ function test_findCallers_onStatementInput() {
 function test_findCallers_multipleStacks() {
   var xml = '<xml xmlns="http://www.w3.org/1999/xhtml">' +
     '<block type="foo" id="test_1"></block>' +
-    '<block type="procedure_callnoreturn" id="test_2"></block>'+
+    '<block type="procedures_callnoreturn" id="test_2"></block>'+
     '<block type="foo" id="test_1"></block>' +
   '</xml>';
   procedureTest_setUp();
@@ -230,8 +230,8 @@ function test_findCallers_multipleStacks() {
 
 function test_findCallers_multipleCallers() {
   var xml = '<xml xmlns="http://www.w3.org/1999/xhtml">' +
-    '<block type="procedure_callnoreturn" id="test_1"></block>' +
-    '<block type="procedure_callnoreturn" id="test_2"></block>'+
+    '<block type="procedures_callnoreturn" id="test_1"></block>' +
+    '<block type="procedures_callnoreturn" id="test_2"></block>'+
   '</xml>';
   procedureTest_setUp();
   try {
@@ -251,3 +251,80 @@ function test_findCallers_multipleCallers() {
     procedureTest_tearDown();
   }
 }
+
+function test_deleteProcedure_noCallers() {
+  // If there are no callers, the stack should be deleted.
+  procedureTest_setUp();
+  var xml = '<xml xmlns="http://www.w3.org/1999/xhtml">' +
+    '<block type="procedures_callnoreturn" id="test_1" x="301" y="516"></block>' +
+    '<block type="foo" id="test_2"></block>' +
+    '<block type="foo" id="test_3"></block>' +
+    '</block>' +
+  '</xml>';
+  try {
+    Blockly.Xml.domToWorkspace(Blockly.Xml.textToDom(xml), workspace);
+    workspace.getBlockById('test_1').procCode_ = 'test_procedure';
+    var rootBlock = workspace.getBlockById('test_1');
+    assertTrue(Blockly.Procedures.deleteProcedureDefCallback('test_procedure',
+        rootBlock));
+    // The other two blocks should stick around.
+    assertEquals(2, workspace.getTopBlocks().length);
+  }
+  finally {
+    procedureTest_tearDown();
+  }
+}
+
+function test_deleteProcedure_recursiveCaller() {
+  // If there is a caller but it's a part of stack starting with definitionRoot,
+  // the stack should be deleted.
+
+  procedureTest_setUp();
+
+  var xml = '<xml xmlns="http://www.w3.org/1999/xhtml">' +
+    '<block type="loop" id="test_1">' +
+      '<statement name="SUBSTACK">' +
+        '<block type="foo" id="test_2">' +
+          '<next>' +
+            '<block type="procedures_callnoreturn" id="test_3"></block>' +
+          '</next></block>' +
+      '</statement>' +
+    '</block>' +
+    '</block>' +
+  '</xml>';
+  try {
+    Blockly.Xml.domToWorkspace(Blockly.Xml.textToDom(xml), workspace);
+    workspace.getBlockById('test_3').procCode_ = 'test_procedure';
+    var rootBlock = workspace.getBlockById('test_1');
+    assertTrue(Blockly.Procedures.deleteProcedureDefCallback('test_procedure',
+        rootBlock));
+    assertEquals(0, workspace.getTopBlocks().length);
+  }
+  finally {
+    procedureTest_tearDown();
+  }
+}
+
+function test_deleteProcedure_nonRecursiveCaller() {
+  // If there is a caller and it's not part of the procedure definition, the
+  // stack should not be deleted.
+
+  procedureTest_setUp();
+  var xml = '<xml xmlns="http://www.w3.org/1999/xhtml">' +
+    '<block type="procedures_callnoreturn" id="test_1" x="301" y="516"></block>' +
+    '<block type="foo" id="test_2"></block>' +
+    '<block type="foo" id="test_3"></block>' +
+  '</xml>';
+  try {
+    Blockly.Xml.domToWorkspace(Blockly.Xml.textToDom(xml), workspace);
+    workspace.getBlockById('test_1').procCode_ = 'test_procedure';
+    var rootBlock = workspace.getBlockById('test_2');
+    assertFalse(Blockly.Procedures.deleteProcedureDefCallback('test_procedure',
+        rootBlock));
+    // All blocks should stay on the workspace.
+    assertEquals(3, workspace.getTopBlocks().length);
+  }
+  finally {
+    procedureTest_tearDown();
+  }
+}