Bug 69731 - [CheatSheet] Missing workflow: running an action should auto-confirm the item

14 files changed, 157 insertions, 4 deletions

diff --git a/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/AllParserTests.java b/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/AllParserTests.java
index 62527fd65..35a95a19f 100644
--- a/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/AllParserTests.java
+++ b/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/AllParserTests.java
@@ -33,5 +33,6 @@ public class AllParserTests extends TestSuite {
 		addTest(TolerateTest.suite());
 		addTestSuite(InvalidCheatsheet.class);
 		addTestSuite(ParseFromString.class);
+		addTestSuite(NoError.class);
 	}
 }
diff --git a/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/InvalidCheatsheet.java b/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/InvalidCheatsheet.java
index f0525ad89..0991f97df 100644
--- a/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/InvalidCheatsheet.java
+++ b/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/InvalidCheatsheet.java
@@ -249,5 +249,12 @@ public class InvalidCheatsheet extends TestCase {
 		assertEquals(IStatus.ERROR, parser.getStatus().getSeverity());
 		StatusCheck.assertStatusContains(parser.getStatus(), "more than one");
 	}
+	
+	public void testConfirmTrueRequiredFalse() {
+		ICheatSheet model = parseTestFile("ConfirmTrueRequiredFalse.xml");
+		assertNull(model);
+		assertEquals(IStatus.ERROR, parser.getStatus().getSeverity());
+		StatusCheck.assertStatusContains(parser.getStatus(), "required = false and confirm = true");
+	}
 
 }
diff --git a/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/NoError.java b/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/NoError.java
new file mode 100644
index 000000000..5900dd1a0
--- /dev/null
+++ b/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/parser/NoError.java
@@ -0,0 +1,47 @@
+/*******************************************************************************
+ * Copyright (c) 2007 IBM Corporation and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+
+/**
+ * Tests which should generate no error or warning when parsing
+ */
+
+package org.eclipse.ua.tests.cheatsheet.parser;
+
+import java.net.URL;
+
+import junit.framework.TestCase;
+
+import org.eclipse.ua.tests.plugin.UserAssistanceTestPlugin;
+import org.eclipse.ua.tests.util.ResourceFinder;
+import org.eclipse.ui.internal.cheatsheets.data.CheatSheetParser;
+import org.eclipse.ui.internal.cheatsheets.data.ICheatSheet;
+
+public class NoError extends TestCase {
+	private static final String NO_ERROR_FOLDER = "data/cheatsheet/no_error/";
+	private CheatSheetParser parser;
+	
+	protected void setUp() throws Exception {
+	    parser = new CheatSheetParser();
+	}
+	
+	private ICheatSheet parseTestFile(String path) {
+		URL testURL = ResourceFinder.findFile(UserAssistanceTestPlugin.getDefault(), 
+					       NO_ERROR_FOLDER + path);
+		return parser.parse(testURL, UserAssistanceTestPlugin.getPluginId(), CheatSheetParser.SIMPLE_ONLY);
+	}
+	
+	public void testConfirmRequiredCombinations() {
+		ICheatSheet model = parseTestFile("ConfirmRequired.xml");
+		assertNotNull(model);
+		assertTrue(parser.getStatus().isOK());
+	}
+
+}
diff --git a/org.eclipse.ua.tests/data/cheatsheet/invalid/ConfirmTrueRequiredFalse.xml b/org.eclipse.ua.tests/data/cheatsheet/invalid/ConfirmTrueRequiredFalse.xml
new file mode 100644
index 000000000..ef6574b70
--- /dev/null
+++ b/org.eclipse.ua.tests/data/cheatsheet/invalid/ConfirmTrueRequiredFalse.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<cheatsheet title="Test confirm = true and required = false">
+  <intro>
+    <description>Test cheat sheet</description>
+  </intro> 
+  <item title="New Item">
+    <description>Item </description>
+    <action class="action_class" pluginId="action_plugin_id" 
+     confirm = "true" required = "false" /> 
+  </item>
+</cheatsheet>
\ No newline at end of file
diff --git a/org.eclipse.ua.tests/data/cheatsheet/no_error/ConfirmRequired.xml b/org.eclipse.ua.tests/data/cheatsheet/no_error/ConfirmRequired.xml
new file mode 100644
index 000000000..164ee8053
--- /dev/null
+++ b/org.eclipse.ua.tests/data/cheatsheet/no_error/ConfirmRequired.xml
@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<cheatsheet title="Test valid permutations of confirm and required attributes">
+  <intro>
+    <description>Test cheat sheet</description>
+  </intro> 
+  <item title="New Item">
+    <description>Item </description>
+    <command serialization="org.eclipse.jdt.ui.PackageExplorer"
+      required = "false" /> 
+  </item>
+  <item title="New Item">
+    <description>Item </description>
+    <action class="action_class" pluginId="action_plugin_id" 
+      required = "true" /> 
+  </item>
+  <item title="New Item">
+    <description>Item </description>
+    <command serialization="org.eclipse.jdt.ui.PackageExplorer" 
+     confirm = "true" required = "true" /> 
+  </item>
+  <item title="New Item">
+    <description>Item </description>
+    <action class="action_class" pluginId="action_plugin_id" 
+     confirm = "true" /> 
+  </item>
+  <item title="New Item">
+    <description>Item </description>
+    <command serialization="org.eclipse.jdt.ui.PackageExplorer" 
+     confirm = "false" required = "false" /> 
+  </item>
+  <item title="New Item">
+    <description>Item </description>
+    <action class="action_class" pluginId="action_plugin_id" 
+     confirm = "false" required = "true" /> 
+  </item>
+  <item title="New Item">
+    <description>Item </description>
+    <command serialization="org.eclipse.jdt.ui.PackageExplorer"
+     confirm = "false"  /> 
+  </item>
+</cheatsheet>
\ No newline at end of file
diff --git a/org.eclipse.ui.cheatsheets/schema/cheatSheetContentFileSpec.exsd b/org.eclipse.ui.cheatsheets/schema/cheatSheetContentFileSpec.exsd
index 5aad2603b..562e3dc2b 100644
--- a/org.eclipse.ui.cheatsheets/schema/cheatSheetContentFileSpec.exsd
+++ b/org.eclipse.ui.cheatsheets/schema/cheatSheetContentFileSpec.exsd
@@ -6,7 +6,10 @@
          <meta.schema plugin="org.eclipse.ui.cheatsheets" id="cheat_sheet_schema" name="Cheat Sheet Content File XML Format"/>
       </appInfo>
       <documentation>
-         The schema definition for a cheat sheet content file. A cheat sheet consists of a series of items (steps) which must be completed in order. Items can be divided into subitems and can launch commands or actions which will perform some of the steps for the user.
+         &lt;p&gt;This document describes the cheat sheet content file structure as a series of
+DTD fragments (&lt;a href=&quot;cheatSheetContentFileSpec.exsd&quot;&gt;machine readable XML schema&lt;/a&gt;).&lt;/p&gt;
+
+&lt;p&gt;A cheat sheet consists of a series of items (steps) which must be completed in order. Items can be divided into subitems and can launch commands or actions which will perform some of the steps for the user.&lt;/p&gt;
       </documentation>
    </annotation>
 
@@ -395,6 +398,13 @@ then the item expands to something equivalent to:
                </documentation>
             </annotation>
          </attribute>
+         <attribute name="required" type="boolean" use="default" value="true">
+            <annotation>
+               <documentation>
+                  if &quot;true&quot; this item or subitem can only be completed by performing this action (it may still be skipped if skip=&quot;true&quot;). If &quot;false&quot; two buttons will be created, one to perform the task and one to mark it as complete, either will complete this step or substep.
+               </documentation>
+            </annotation>
+         </attribute>
       </complexType>
    </element>
 
@@ -453,6 +463,13 @@ stores the result in the cheat sheet variable &amp;quot;result&amp;quot;.&lt;/p&
                </documentation>
             </annotation>
          </attribute>
+         <attribute name="required" type="boolean" use="default" value="true">
+            <annotation>
+               <documentation>
+                  if &quot;true&quot; this item or subitem can only be completed by performing this command (it may still be skipped if skip=&quot;true&quot;). If &quot;false&quot; two buttons will be created, one to perform the task and one to mark it as complete, either will complete this step or substep.
+               </documentation>
+            </annotation>
+         </attribute>
       </complexType>
    </element>
 
diff --git a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/Messages.java b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/Messages.java
index c99ea8433..8308732f5 100644
--- a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/Messages.java
+++ b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/Messages.java
@@ -88,6 +88,7 @@ public final class Messages extends NLS {
 	public static String ERROR_PARSING_NO_SERIALIZATION;
 	public static String ERROR_PARSING_INCOMPATIBLE_CHILDREN;
 	public static String ERROR_PARSING_DUPLICATE_CHILD;
+	public static String ERROR_PARSING_REQUIRED_CONFIRM;
 	public static String ERROR_COMMAND_ID_NOT_FOUND;
 	public static String ERROR_COMMAND_ERROR_STATUS;
 	public static String ERROR_COMMAND_SERVICE_UNAVAILABLE;
diff --git a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/Messages.properties b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/Messages.properties
index fe30f2bd9..3bb105d68 100644
--- a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/Messages.properties
+++ b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/Messages.properties
@@ -124,6 +124,7 @@ ERROR_PARSING_NO_VALUES = The ''{0}'' element must specify a values attribute.
 ERROR_PARSING_NO_LABEL = The ''{0}'' element must specify a label attribute.
 ERROR_PARSING_INCOMPATIBLE_CHILDREN = The ''{0}'' element contains incompatible child elements ''{1}'' and ''{2}''.
 ERROR_PARSING_DUPLICATE_CHILD = The ''{0}'' element has more than one child element ''{1}''.
+ERROR_PARSING_REQUIRED_CONFIRM = The ''{0}'' element has both required = false and confirm = true.
 
 # CheatsheetCommand, content error messages
 ERROR_PARSING_NO_SERIALIZATION = The ''{0}'' element must specify a serialization attribute.
diff --git a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/AbstractExecutable.java b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/AbstractExecutable.java
index 3025b5e01..e2c1009c8 100644
--- a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/AbstractExecutable.java
+++ b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/AbstractExecutable.java
@@ -23,8 +23,9 @@ import org.w3c.dom.Node;
 public abstract class AbstractExecutable {
 	
 	private String[] params;
-	private boolean confirm;
+	private boolean confirm = false;
 	private String when;
+	private boolean required = true;
 	
 	/**
 	 * This method returns an array of parameters specified to be passed to the action class
@@ -50,6 +51,14 @@ public abstract class AbstractExecutable {
 	public boolean isConfirm() {
 		return confirm;
 	}
+	
+	/**
+	 * @return true if this step or substep can only be completed by performing
+	 * this executable.
+	 */
+	public boolean isRequired() {
+		return required;
+	}
 
 	/**
 	 * This method allows you to set the string parameters to be passed to the action class on running it 
@@ -67,6 +76,15 @@ public abstract class AbstractExecutable {
 	public void setConfirm(boolean value) {
 		this.confirm = value;
 	}
+	
+	/**
+	 * Set whether this executable can be by passed.
+	 * @param required if true this action must be performed to complete this 
+	 * step or substep.
+	 */
+	public void setRequired(boolean required) {
+		this.required = required;		
+	}
 
 	/**
 	 * Indicates this action is to be used if and only if the value of the condition attribute
diff --git a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/Action.java b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/Action.java
index c983dd208..98e43b92c 100644
--- a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/Action.java
+++ b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/Action.java
@@ -85,6 +85,9 @@ public class Action extends AbstractExecutable {
 		if(!hasPluginId) {
 			return NLS.bind(Messages.ERROR_PARSING_NO_PLUGINID, (new Object[] {node.getNodeName()}));
 		}
+		if(isConfirm() && !isRequired()) {
+			return NLS.bind(Messages.ERROR_PARSING_REQUIRED_CONFIRM, (new Object[] {node.getNodeName()}));
+		}
 		return null;
 	}
 
diff --git a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/CheatSheetCommand.java b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/CheatSheetCommand.java
index 4eba701d9..f44931e62 100644
--- a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/CheatSheetCommand.java
+++ b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/CheatSheetCommand.java
@@ -64,6 +64,9 @@ public class CheatSheetCommand extends AbstractExecutable {
 		if(!serializationFound) {
 			return NLS.bind(Messages.ERROR_PARSING_NO_SERIALIZATION, (new Object[] {node.getNodeName()}));
 		}
+		if(isConfirm() && !isRequired()) {
+			return NLS.bind(Messages.ERROR_PARSING_REQUIRED_CONFIRM, (new Object[] {node.getNodeName()}));
+		}
 		return null;
 	}
 
diff --git a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/CheatSheetParser.java b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/CheatSheetParser.java
index 6eb78c551..b326a5c26 100644
--- a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/CheatSheetParser.java
+++ b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/CheatSheetParser.java
@@ -226,6 +226,8 @@ public class CheatSheetParser implements IStatusContainer {
 					executable.setConfirm(attribute.getNodeValue().equals(TRUE_STRING));} 
 				else if (attributeName.equals(IParserTags.WHEN)) {
 					executable.setWhen(attribute.getNodeValue());
+				} else if (attributeName.equals(IParserTags.REQUIRED)) {
+					executable.setRequired(attribute.getNodeValue().equals(TRUE_STRING));
 				} else if (executable.hasParams() && attributeName.startsWith(IParserTags.PARAM)) {
 					try {
 						if(params == null) {
diff --git a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/IParserTags.java b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/IParserTags.java
index 5b30ae1d7..ee5aaafe7 100644
--- a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/IParserTags.java
+++ b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/data/IParserTags.java
@@ -127,6 +127,7 @@ public interface IParserTags {
 	public static final String RETURNS = "returns"; //$NON-NLS-1$
 	public static final String ON_COMPLETION = "onCompletion"; //$NON-NLS-1$
 	public static final String CONTENT_URL = "contentURL"; //$NON-NLS-1$
+	public static final String REQUIRED = "required"; //$NON-NLS-1$
 
 	// Constants for re-escaping XML characters
 	public static final String LESS_THAN = "&lt;"; //$NON-NLS-1$
diff --git a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/views/CoreItem.java b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/views/CoreItem.java
index 5d5113e4c..9b0208291 100644
--- a/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/views/CoreItem.java
+++ b/org.eclipse.ui.cheatsheets/src/org/eclipse/ui/internal/cheatsheets/views/CoreItem.java
@@ -90,7 +90,7 @@ public class CoreItem extends ViewItem {
 				}
 			});
 		}
-		if (!isActionShown || executable.isConfirm()) {
+		if (!isActionShown || executable.isConfirm() || !executable.isRequired()) {
 			final ImageHyperlink completeButton = createButtonWithText(buttonComposite, CheatSheetPlugin.getPlugin().getImage(ICheatSheetResource.CHEATSHEET_ITEM_BUTTON_COMPLETE), this, itemColor, Messages.COMPLETE_TASK_TOOLTIP);
 			page.getToolkit().adapt(completeButton, true, true);
 			completeButton.addHyperlinkListener(new HyperlinkAdapter() {
@@ -193,7 +193,7 @@ public class CoreItem extends ViewItem {
 			});
 			holder.setStartButton(startButton);
 		}
-		if (!isActionShown || subExecutable.isConfirm()) {
+		if (!isActionShown || subExecutable.isConfirm() || !subExecutable.isRequired()) {
 			added++;
 			final ImageHyperlink completeButton = createButton(buttonComposite, CheatSheetPlugin.getPlugin().getImage(ICheatSheetResource.CHEATSHEET_ITEM_BUTTON_COMPLETE), this, itemColor, Messages.COMPLETE_TASK_TOOLTIP);
 			page.getToolkit().adapt(completeButton, true, true);