Fix up non-API TerminalServiceRunnable to increase code readability

Removes `isExecuteAsync` and returning `IStatus` from `run`. Both
of these are not actually used and make the code more complicated
to read and maintain. These changes then make it possible to change
TerminalServiceRunnable into a `@FunctionalInterface` and use lambdas

Author: jonahgraham

terminal/bundles/org.eclipse.terminal.view.ui/src/org/eclipse/terminal/view/ui/internal/TerminalService.java

@@ -16,12 +16,12 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
 
 import org.eclipse.core.runtime.Assert;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.ILog;
 import org.eclipse.core.runtime.ISafeRunnable;
-import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.ListenerList;
 import org.eclipse.core.runtime.SafeRunner;
 import org.eclipse.core.runtime.Status;
@@ -68,8 +68,11 @@ public class TerminalService implements ITerminalService {
 
 	/**
 	 * Common terminal service runnable implementation.
+	 *
+	 * These runnables are run asynchronously on the UI thread.
 	 */
-	protected static abstract class TerminalServiceRunnable {
+	@FunctionalInterface
+	private static interface TerminalServiceRunnable {
 
 		/**
 		 * Invoked to execute the terminal service runnable.
@@ -78,21 +81,10 @@ protected static abstract class TerminalServiceRunnable {
 		 * @param title The terminal tab title. Must not be <code>null</code>.
 		 * @param connector The terminal connector. Must not be <code>null</code>.
 		 * @param data The custom terminal data node or <code>null</code>.
-		 * @return the result {@link IStatus}
-		 */
-		public abstract IStatus run(TerminalViewId tvid, String title, ITerminalConnector connector, Object data);
-
-		/**
-		 * Returns if or if not to execute the runnable asynchronously.
-		 * <p>
-		 * The method returns per default <code>true</code>. Overwrite to
-		 * modify the behavior.
-		 *
-		 * @return <code>True</code> to execute the runnable asynchronously, <code>false</code> otherwise.
+		 * @throws CoreException if operation does not complete successfully
+		 * @throws NullPointerException if any required arguments are null.
 		 */
-		public boolean isExecuteAsync() {
-			return true;
-		}
+		void run(TerminalViewId tvid, String title, ITerminalConnector connector, Object data) throws CoreException;
 	}
 
 	@Activate
@@ -194,18 +186,14 @@ protected final CompletableFuture<?> executeServiceOperation(final Map<String, O
 	private CompletableFuture<?> executeServiceOperation(final TerminalServiceRunnable runnable, TerminalViewId tvid,
 			final String title, final ITerminalConnector connector, final Object data) {
 		try { // Execute the operation
-			if (runnable.isExecuteAsync()) {
-				return CompletableFuture.supplyAsync(() -> runnable.run(tvid, title, connector, data),
-						/*
-						 * A special executor is used here because the default Display executor will
-						 * do a syncExec here if called from the UI thread, but isExecuteAsync is true so we
-						 * must run async on the UI thread.
-						 */
-						Display.getDefault()::asyncExec);
-
-			} else {
-				return CompletableFuture.completedFuture(runnable.run(tvid, title, connector, data));
-			}
+			return CompletableFuture.runAsync(() -> {
+				try {
+					runnable.run(tvid, title, connector, data);
+				} catch (CoreException e) {
+					throw new CompletionException(e);
+				}
+			}, Display.getDefault()::asyncExec);
+
 		} catch (RuntimeException e) {
 			return CompletableFuture.failedFuture(e);
 		}
@@ -265,9 +253,10 @@ public CompletableFuture<?> openConsole(final Map<String, Object> properties) {
 		final boolean restoringView = fRestoringView;
 		return executeServiceOperation(properties, new TerminalServiceRunnable() {
 			@Override
-			public IStatus run(TerminalViewId tvid, String title, ITerminalConnector connector, Object data) {
+			public void run(TerminalViewId tvid, String title, ITerminalConnector connector, Object data)
+					throws CoreException {
 				if (restoringView) {
-					return doRun(tvid, title, connector, data);
+					doRun(tvid, title, connector, data);
 				} else {
 					// First, restore the view. This opens consoles from the memento
 					fRestoringView = true;
@@ -277,11 +266,12 @@ public IStatus run(TerminalViewId tvid, String title, ITerminalConnector connect
 						ILog.get().log(e.getStatus());
 					}
 					fRestoringView = false;
-					return doRun(tvid, title, connector, data);
+					doRun(tvid, title, connector, data);
 				}
 			}
 
-			private IStatus doRun(TerminalViewId tvid, String title, ITerminalConnector connector, Object data) {
+			private void doRun(TerminalViewId tvid, String title, ITerminalConnector connector, Object data)
+					throws CoreException {
 				// Determine the terminal encoding
 				String encoding = (String) properties.get(ITerminalsConnectorConstants.PROP_ENCODING);
 				// Create the flags to pass on to openConsole
@@ -302,16 +292,11 @@ private IStatus doRun(TerminalViewId tvid, String title, ITerminalConnector conn
 					flags.put(ITerminalsConnectorConstants.PROP_TITLE_DISABLE_ANSI_TITLE, false);
 				}
 				// Open the new console
-				try {
-					Widget console = consoleViewManager.openConsole(tvid, title, encoding, connector, data, flags);
-					// Associate the original terminal properties with the tab item.
-					// This makes it easier to persist the connection data within the memento handler
-					if (!console.isDisposed()) {
-						console.setData("properties", properties); //$NON-NLS-1$
-					}
-					return Status.OK_STATUS;
-				} catch (CoreException e) {
-					return e.getStatus();
+				Widget console = consoleViewManager.openConsole(tvid, title, encoding, connector, data, flags);
+				// Associate the original terminal properties with the tab item.
+				// This makes it easier to persist the connection data within the memento handler
+				if (!console.isDisposed()) {
+					console.setData("properties", properties); //$NON-NLS-1$
 				}
 			}
 		});
@@ -320,25 +305,12 @@ private IStatus doRun(TerminalViewId tvid, String title, ITerminalConnector conn
 	@Override
 	public CompletableFuture<?> closeConsole(Map<String, Object> properties) {
 		Assert.isNotNull(properties);
-		return executeServiceOperation(properties, new TerminalServiceRunnable() {
-			@Override
-			public IStatus run(TerminalViewId tvid, String title, ITerminalConnector connector, Object data) {
-				consoleViewManager.closeConsole(tvid, title, connector, data);
-				return Status.OK_STATUS;
-			}
-		});
+		return executeServiceOperation(properties, consoleViewManager::closeConsole);
 	}
 
 	@Override
 	public CompletableFuture<?> terminateConsole(Map<String, Object> properties) {
 		Assert.isNotNull(properties);
-
-		return executeServiceOperation(properties, new TerminalServiceRunnable() {
-			@Override
-			public IStatus run(TerminalViewId tvid, String title, ITerminalConnector connector, Object data) {
-				consoleViewManager.terminateConsole(tvid, title, connector, data);
-				return Status.OK_STATUS;
-			}
-		});
+		return executeServiceOperation(properties, consoleViewManager::terminateConsole);
 	}
 }