Bug 573712: Reorganize external tools detection code

The code was not very extensible and the main purpose of the bug
is to add additional detectors. This first commit refactors
the code to make that easier, and to provide a platform for
what could become an extension point.

Change-Id: I5ce514eda11f2573098d6e16663e324954da961b

terminal/plugins/org.eclipse.tm.terminal.view.ui/META-INF/MANIFEST.MF

@@ -32,6 +32,7 @@ Export-Package: org.eclipse.tm.terminal.view.ui.actions,
  org.eclipse.tm.terminal.view.ui.launcher,
  org.eclipse.tm.terminal.view.ui.listeners,
  org.eclipse.tm.terminal.view.ui.local.showin;x-internal:=true,
+ org.eclipse.tm.terminal.view.ui.local.showin.detectors;x-internal:=true,
  org.eclipse.tm.terminal.view.ui.manager,
  org.eclipse.tm.terminal.view.ui.nls;x-internal:=true,
  org.eclipse.tm.terminal.view.ui.panels,

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/DynamicContributionItems.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2014, 2018 Wind River Systems, Inc. and others. All rights reserved.
+ * Copyright (c) 2014, 2021 Wind River Systems, Inc. and others. All rights reserved.
  * This program and the accompanying materials are made available under the terms
  * of the Eclipse Public License 2.0 which accompanies this distribution, and is
  * available at https://www.eclipse.org/legal/epl-2.0/
@@ -71,7 +71,7 @@ public class DynamicContributionItems extends CompoundContributionItem implement
 					if (name != null && !"".equals(name) && path != null && !"".equals(path)) { //$NON-NLS-1$ //$NON-NLS-2$
 						IAction action = createAction(name, path, args, translate);
 
-						ImageData id = icon != null ? ExternalExecutablesManager.loadImage(icon) : null;
+						ImageData id = icon != null ? ExternalExecutablesUtils.loadImage(icon) : null;
 						if (id != null) {
 							ImageDescriptor desc = ImageDescriptor.createFromImageData(id);
 							if (desc != null)

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/ExternalExecutablesManager.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2014, 2018 Wind River Systems, Inc. and others. All rights reserved.
+ * Copyright (c) 2014, 2021 Wind River Systems, Inc. and others. All rights reserved.
  * This program and the accompanying materials are made available under the terms
  * of the Eclipse Public License 2.0 which accompanies this distribution, and is
  * available at https://www.eclipse.org/legal/epl-2.0/
@@ -23,16 +23,14 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Properties;
-import java.util.StringTokenizer;
+import java.util.stream.Collectors;
 
 import org.eclipse.core.runtime.Assert;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.Platform;
-import org.eclipse.swt.graphics.ImageData;
-import org.eclipse.swt.graphics.ImageLoader;
 import org.eclipse.tm.terminal.view.ui.activator.UIPlugin;
-import org.eclipse.tm.terminal.view.ui.interfaces.IExternalExecutablesProperties;
 import org.eclipse.tm.terminal.view.ui.internal.ExternalExecutablesState;
+import org.eclipse.tm.terminal.view.ui.local.showin.detectors.DetectGitBash;
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.services.ISourceProviderService;
 
@@ -40,8 +38,8 @@ import org.eclipse.ui.services.ISourceProviderService;
  * External executables manager implementation.
  */
 public class ExternalExecutablesManager {
-	// Flag to indicate if we have searched for git bash already
+	// XXX: This may make a useful extension point?
-	private static boolean gitBashSearchDone = false;
+	private static List<IDetectExternalExecutable> detectors = List.of(new DetectGitBash());
 
 	/**
 	 * Loads the list of all saved external executables.
@@ -49,7 +47,7 @@ public class ExternalExecutablesManager {
 	 * @return The list of all saved external executables or <code>null</code>.
 	 */
 	public static List<Map<String, String>> load() {
-		List<Map<String, String>> l = new ArrayList<Map<String, String>>();
+		List<Map<String, String>> externalExecutables = new ArrayList<>();
 
 		IPath stateLocation = UIPlugin.getDefault().getStateLocation();
 		if (stateLocation != null) {
@@ -62,7 +60,7 @@ public class ExternalExecutablesManager {
 					r = new FileReader(f);
 					data.load(r);
 
-					Map<Integer, Map<String, String>> c = new HashMap<Integer, Map<String, String>>();
+					Map<Integer, Map<String, String>> c = new HashMap<>();
 					for (String name : data.stringPropertyNames()) {
 						if (name == null || name.indexOf('.') == -1)
 							continue;
@@ -82,7 +80,7 @@ public class ExternalExecutablesManager {
 
 						Map<String, String> m = c.get(i);
 						if (m == null) {
-							m = new HashMap<String, String>();
+							m = new HashMap<>();
 							c.put(i, m);
 						}
 						Assert.isNotNull(m);
@@ -90,12 +88,12 @@ public class ExternalExecutablesManager {
 						m.put(k, data.getProperty(name));
 					}
 
-					List<Integer> k = new ArrayList<Integer>(c.keySet());
+					List<Integer> k = new ArrayList<>(c.keySet());
 					Collections.sort(k);
 					for (Integer i : k) {
 						Map<String, String> m = c.get(i);
 						if (m != null && !m.isEmpty())
-							l.add(m);
+							externalExecutables.add(m);
 					}
 				} catch (Exception e) {
 					if (Platform.inDebugMode()) {
@@ -106,111 +104,30 @@ public class ExternalExecutablesManager {
 						try {
 							r.close();
 						} catch (IOException e) {
-							/* ignored on purpose */ }
+							/* ignored on purpose */
-				}
-			}
-		}
-
-		// Lookup git bash (Windows Hosts only)
-		if (!gitBashSearchDone && Platform.OS_WIN32.equals(Platform.getOS())) {
-			// Check the existing entries first
-			// Find a entry labeled "Git Bash"
-			Map<String, String> m = null;
-			for (Map<String, String> candidate : l) {
-				String name = candidate.get(IExternalExecutablesProperties.PROP_NAME);
-				if ("Git Bash".equals(name)) { //$NON-NLS-1$
-					m = candidate;
-					break;
-				}
-			}
-
-			// If not found in the existing entries, check the path
-			if (m == null) {
-				String gitPath = null;
-				String iconPath = null;
-
-				String path = System.getenv("PATH"); //$NON-NLS-1$
-				if (path != null) {
-					StringTokenizer tokenizer = new StringTokenizer(path, ";"); //$NON-NLS-1$
-					while (tokenizer.hasMoreTokens()) {
-						String token = tokenizer.nextToken();
-						File f = new File(token, "git.exe"); //$NON-NLS-1$
-						if (f.canRead()) {
-							File f2 = new File(f.getParentFile().getParentFile(), "bin/sh.exe"); //$NON-NLS-1$
-							if (f2.canExecute()) {
-								gitPath = f2.getAbsolutePath();
-							}
-
-							iconPath = getGitIconPath(f.getParentFile().getParentFile());
-
-							break;
 						}
-					}
-				}
-
-				// if it is not found in the PATH, check the default install locations
-				if (gitPath == null) {
-					File f = new File("C:/Program Files (x86)/Git/bin/sh.exe"); //$NON-NLS-1$
-					if (!f.exists()) {
-						f = new File("C:/Program Files/Git/bin/sh.exe"); //$NON-NLS-1$
-					}
-
-					if (f.exists() && f.canExecute()) {
-						gitPath = f.getAbsolutePath();
-						iconPath = getGitIconPath(f.getParentFile().getParentFile());
-					}
-				}
-
-				if (gitPath != null) {
-					m = new HashMap<String, String>();
-					m.put(IExternalExecutablesProperties.PROP_NAME, "Git Bash"); //$NON-NLS-1$
-					m.put(IExternalExecutablesProperties.PROP_PATH, gitPath);
-					m.put(IExternalExecutablesProperties.PROP_ARGS, "--login -i"); //$NON-NLS-1$
-					if (iconPath != null)
-						m.put(IExternalExecutablesProperties.PROP_ICON, iconPath);
-					m.put(IExternalExecutablesProperties.PROP_TRANSLATE, Boolean.TRUE.toString());
-
-					l.add(m);
-					save(l);
 				}
 			}
-
-			// Do not search again for git bash while the session is running
-			gitBashSearchDone = true;
 		}
 
-		return l;
+		var readOnly = Collections.unmodifiableList(externalExecutables);
-	}
+		var detected = detectors.stream().flatMap(detector -> detector.run(readOnly).stream())
-
+				.collect(Collectors.toList());
-	private static String getGitIconPath(File parent) {
+		if (!detected.isEmpty()) {
-		File f = new File(parent, "etc/git.ico"); //$NON-NLS-1$
+			externalExecutables.addAll(detected);
-		if (f.canRead()) {
+			save(externalExecutables);
-			return f.getAbsolutePath();
 		}
 
-		// check for icon in newer versions of Git for Windows 32 bit
+		return externalExecutables;
-		f = new File(parent, "mingw32/share/git/git-for-windows.ico"); //$NON-NLS-1$
-		if (f.canRead()) {
-			return f.getAbsolutePath();
-		}
-
-		// check for icon in newer versions of Git for Windows 64 bit
-		f = new File(parent, "mingw64/share/git/git-for-windows.ico"); //$NON-NLS-1$
-		if (f.canRead()) {
-			return f.getAbsolutePath();
-		}
-
-		return null;
 	}
 
 	/**
 	 * Saves the list of external executables.
 	 *
-	 * @param l The list of external executables or <code>null</code>.
+	 * @param externalExecutables The list of external executables or <code>null</code>.
 	 */
-	@SuppressWarnings("cast")
+	public static void save(List<Map<String, String>> externalExecutables) {
-	public static void save(List<Map<String, String>> l) {
+		ISourceProviderService sourceProviderService = PlatformUI.getWorkbench()
-		ISourceProviderService sourceProviderService = (ISourceProviderService) PlatformUI.getWorkbench()
 				.getService(ISourceProviderService.class);
 		ExternalExecutablesState stateService = (ExternalExecutablesState) sourceProviderService
 				.getSourceProvider(ExternalExecutablesState.CONFIGURED_STATE);
@@ -218,7 +135,7 @@ public class ExternalExecutablesManager {
 		IPath stateLocation = UIPlugin.getDefault().getStateLocation();
 		if (stateLocation != null) {
 			File f = stateLocation.append(".executables/data.properties").toFile(); //$NON-NLS-1$
-			if (f.isFile() && (l == null || l.isEmpty())) {
+			if (f.isFile() && (externalExecutables == null || externalExecutables.isEmpty())) {
 				@SuppressWarnings("unused")
 				boolean s = f.delete();
 
@@ -229,8 +146,8 @@ public class ExternalExecutablesManager {
 
 				try {
 					Properties data = new Properties();
-					for (int i = 0; i < l.size(); i++) {
+					for (int i = 0; i < externalExecutables.size(); i++) {
-						Map<String, String> m = l.get(i);
+						Map<String, String> m = externalExecutables.get(i);
 						for (Entry<String, String> e : m.entrySet()) {
 							String key = Integer.toString(i) + "." + e.getKey(); //$NON-NLS-1$
 							data.setProperty(key, e.getValue());
@@ -264,49 +181,4 @@ public class ExternalExecutablesManager {
 			}
 		}
 	}
-
-	/**
-	 * Loads the image data suitable for showing an icon in a menu
-	 * (16 x 16, 8bit depth) from the given file.
-	 *
-	 * @param path The image file path. Must not be <code>null</code>.
-	 * @return The image data or <code>null</code>.
-	 */
-	public static ImageData loadImage(String path) {
-		Assert.isNotNull(path);
-
-		ImageData id = null;
-		ImageData biggest = null;
-
-		ImageLoader loader = new ImageLoader();
-		ImageData[] data = loader.load(path);
-
-		if (data != null) {
-			for (ImageData d : data) {
-				if (d.height == 16 && d.width == 16) {
-					if (id == null || id.height != 16 && id.width != 16) {
-						id = d;
-					} else if (d.depth < id.depth && d.depth >= 8) {
-						id = d;
-					}
-				} else {
-					if (id == null) {
-						id = d;
-						biggest = d;
-					} else if (id.height != 16 && d.height < id.height && id.width != 16 && d.width < id.width) {
-						id = d;
-					} else if (biggest == null || d.height > biggest.height && d.width > biggest.width) {
-						biggest = d;
-					}
-				}
-			}
-		}
-
-		// if the icon is still to big -> downscale the biggest
-		if (id != null && id.height > 16 && id.width > 16) {
-			id = biggest.scaledTo(16, 16);
-		}
-
-		return id;
-	}
 }

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/ExternalExecutablesUtils.java

@@ -0,0 +1,85 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Kichwa Coders Canada Inc. and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package org.eclipse.tm.terminal.view.ui.local.showin;
+
+import java.io.File;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.function.Function;
+
+import org.eclipse.core.runtime.Assert;
+import org.eclipse.swt.graphics.ImageData;
+import org.eclipse.swt.graphics.ImageLoader;
+
+public class ExternalExecutablesUtils {
+
+	/**
+	 * Loads the image data suitable for showing an icon in a menu
+	 * (16 x 16, 8bit depth) from the given file.
+	 *
+	 * @param path The image file path. Must not be <code>null</code>.
+	 * @return The image data or <code>null</code>.
+	 */
+	public static ImageData loadImage(String path) {
+		Assert.isNotNull(path);
+
+		ImageData id = null;
+		ImageData biggest = null;
+
+		ImageLoader loader = new ImageLoader();
+		ImageData[] data = loader.load(path);
+
+		if (data != null) {
+			for (ImageData d : data) {
+				if (d.height == 16 && d.width == 16) {
+					if (id == null || id.height != 16 && id.width != 16) {
+						id = d;
+					} else if (d.depth < id.depth && d.depth >= 8) {
+						id = d;
+					}
+				} else {
+					if (id == null) {
+						id = d;
+						biggest = d;
+					} else if (id.height != 16 && d.height < id.height && id.width != 16 && d.width < id.width) {
+						id = d;
+					} else if (biggest == null || d.height > biggest.height && d.width > biggest.width) {
+						biggest = d;
+					}
+				}
+			}
+		}
+
+		// if the icon is still too big -> downscale the biggest
+		if (id != null && id.height > 16 && id.width > 16 && biggest != null) {
+			id = biggest.scaledTo(16, 16);
+		}
+
+		return id;
+	}
+
+	public static <T> Optional<T> visitPATH(Function<String, Optional<T>> r) {
+		String path = System.getenv("PATH"); //$NON-NLS-1$
+		if (path != null) {
+			StringTokenizer tokenizer = new StringTokenizer(path, File.pathSeparator);
+			while (tokenizer.hasMoreTokens()) {
+				String token = tokenizer.nextToken();
+
+				Optional<T> apply = r.apply(token);
+				if (apply.isPresent()) {
+					return apply;
+				}
+			}
+		}
+		return Optional.empty();
+	}
+
+}

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/IDetectExternalExecutable.java

@@ -0,0 +1,35 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Kichwa Coders Canada Inc. and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package org.eclipse.tm.terminal.view.ui.local.showin;
+
+import java.util.List;
+import java.util.Map;
+
+import org.eclipse.tm.terminal.view.ui.interfaces.IExternalExecutablesProperties;
+
+@FunctionalInterface
+public interface IDetectExternalExecutable {
+	/**
+	 * Detect any additional external executables that can be added to the Show In list.
+	 *
+	 * This method is sometimes called in the UI thread when displaying context menus, so should
+	 * either be very fast, or it should use a flag to not re-run multiple times after the initial detection.
+	 *
+	 * The same instance of the {@link IDetectExternalExecutable} will be used on each invocation of this method.
+	 *
+	 * @param externalExecutables is the list of executables already present that can be used to prevent duplicate
+	 * entries. This list should not be modified.
+	 * @return a list of additional items to add to the external executables list. Each map entry should have keys
+	 * that match {@link IExternalExecutablesProperties}. Must not return <code>null</code>, return
+	 * an empty list instead.
+	 */
+	List<Map<String, String>> run(List<Map<String, String>> externalExecutables);
+}

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/local/showin/detectors/DetectGitBash.java

@@ -0,0 +1,107 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Kichwa Coders Canada Inc. and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package org.eclipse.tm.terminal.view.ui.local.showin.detectors;
+
+import java.io.File;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.function.Predicate;
+
+import org.eclipse.core.runtime.Platform;
+import org.eclipse.tm.terminal.view.ui.interfaces.IExternalExecutablesProperties;
+import org.eclipse.tm.terminal.view.ui.local.showin.ExternalExecutablesUtils;
+import org.eclipse.tm.terminal.view.ui.local.showin.IDetectExternalExecutable;
+
+public class DetectGitBash implements IDetectExternalExecutable {
+
+	private static final String GIT_BASH = "Git Bash"; //$NON-NLS-1$
+	private static boolean gitBashSearchDone = false;
+
+	@Override
+	public List<Map<String, String>> run(List<Map<String, String>> externalExecutables) {
+		// Lookup git bash (Windows Hosts only)
+		if (!gitBashSearchDone && Platform.OS_WIN32.equals(Platform.getOS())) {
+			// Do not search again for git bash while the session is running
+			gitBashSearchDone = true;
+
+			// Check the existing entries first
+			// Find a entry labeled "Git Bash"
+			if (externalExecutables.stream().map(m -> m.get(IExternalExecutablesProperties.PROP_NAME))
+					.anyMatch(Predicate.isEqual(GIT_BASH))) {
+				return Collections.emptyList();
+			}
+
+			// If not found in the existing entries, check the path
+			Optional<File> result = ExternalExecutablesUtils.visitPATH(entry -> {
+				File f = new File(entry, "git.exe"); //$NON-NLS-1$
+				if (f.canRead()) {
+					File check = f.getParentFile().getParentFile();
+					if (new File(check, "bin/sh.exe").canExecute()) { //$NON-NLS-1$
+						return Optional.of(check);
+					}
+				}
+				return Optional.empty();
+			});
+			// if it is not found in the PATH, check the default install locations
+			result = result.or(() -> {
+				File f = new File("C:/Program Files (x86)/Git/bin/sh.exe"); //$NON-NLS-1$
+				if (!f.exists()) {
+					f = new File("C:/Program Files/Git/bin/sh.exe"); //$NON-NLS-1$
+				}
+				if (f.exists() && f.canExecute()) {
+					return Optional.of(f.getParentFile().getParentFile());
+				}
+				return Optional.empty();
+			});
+
+			Optional<String> gitPath = result.map(f -> new File(f, "bin/sh.exe").getAbsolutePath()); //$NON-NLS-1$
+			Optional<String> iconPath = result.flatMap(f -> getGitIconPath(f));
+
+			return gitPath.map(path -> {
+				Map<String, String> m = new HashMap<>();
+				m.put(IExternalExecutablesProperties.PROP_NAME, GIT_BASH);
+				m.put(IExternalExecutablesProperties.PROP_PATH, path);
+				m.put(IExternalExecutablesProperties.PROP_ARGS, "--login -i"); //$NON-NLS-1$
+				iconPath.ifPresent(icon -> m.put(IExternalExecutablesProperties.PROP_ICON, icon));
+				m.put(IExternalExecutablesProperties.PROP_TRANSLATE, Boolean.TRUE.toString());
+
+				return List.of(m);
+			}).orElse(Collections.emptyList());
+
+		}
+		return Collections.emptyList();
+
+	}
+
+	private static Optional<String> getGitIconPath(File parent) {
+		File f = new File(parent, "etc/git.ico"); //$NON-NLS-1$
+		if (f.canRead()) {
+			return Optional.of(f.getAbsolutePath());
+		}
+
+		// check for icon in newer versions of Git for Windows 32 bit
+		f = new File(parent, "mingw32/share/git/git-for-windows.ico"); //$NON-NLS-1$
+		if (f.canRead()) {
+			return Optional.of(f.getAbsolutePath());
+		}
+
+		// check for icon in newer versions of Git for Windows 64 bit
+		f = new File(parent, "mingw64/share/git/git-for-windows.ico"); //$NON-NLS-1$
+		if (f.canRead()) {
+			return Optional.of(f.getAbsolutePath());
+		}
+
+		return Optional.empty();
+	}
+}

terminal/plugins/org.eclipse.tm.terminal.view.ui/src/org/eclipse/tm/terminal/view/ui/preferences/PreferencePage.java

@@ -64,6 +64,7 @@ import org.eclipse.tm.terminal.view.ui.activator.UIPlugin;
 import org.eclipse.tm.terminal.view.ui.controls.NoteCompositeHelper;
 import org.eclipse.tm.terminal.view.ui.interfaces.IExternalExecutablesProperties;
 import org.eclipse.tm.terminal.view.ui.interfaces.IPreferenceKeys;
+import org.eclipse.tm.terminal.view.ui.local.showin.ExternalExecutablesUtils;
 import org.eclipse.tm.terminal.view.ui.local.showin.ExternalExecutablesDialog;
 import org.eclipse.tm.terminal.view.ui.local.showin.ExternalExecutablesManager;
 import org.eclipse.tm.terminal.view.ui.nls.Messages;
@@ -524,7 +525,7 @@ public class PreferencePage extends org.eclipse.jface.preference.PreferencePage
 						if (icon != null) {
 							i = images.get(icon);
 							if (i == null) {
-								ImageData id = ExternalExecutablesManager.loadImage(icon);
+								ImageData id = ExternalExecutablesUtils.loadImage(icon);
 								if (id != null) {
 									ImageDescriptor d = ImageDescriptor.createFromImageData(id);
 									if (d != null)