JAL-4485 Better var name and using FileUtils method in StructureManager

diff --git a/src/ext/edu/ucsf/rbvi/strucviz2/StructureManager.java b/src/ext/edu/ucsf/rbvi/strucviz2/StructureManager.java
index 974b493..f10d08e 100644 (file)
--- a/src/ext/edu/ucsf/rbvi/strucviz2/StructureManager.java
+++ b/src/ext/edu/ucsf/rbvi/strucviz2/StructureManager.java
@@ -49,7 +49,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import jalview.bin.Cache;
-import jalview.bin.Console;
 import jalview.gui.Preferences;
 import jalview.util.FileUtils;
 import jalview.util.Platform;
@@ -963,6 +962,8 @@ public class StructureManager
     String chimeraExe = isChimeraX ? "ChimeraX" : "chimera";
     String[] versionsBlacklist = isChimeraX ? CHIMERAX_VERSIONS_BLACKLIST
             : CHIMERA_VERSIONS_BLACKLIST;
+    String[] versionsWhitelist = isChimeraX ? CHIMERAX_VERSIONS
+            : CHIMERA_VERSIONS;
 
     /*
      * Jalview addition: check if path set in user preferences
@@ -1027,136 +1028,31 @@ public class StructureManager
     }
     else if (Platform.isWin())
     {
-      Set<File> installedChimera = new HashSet<>();
-      String[] templates = new String[] { "%s\\%s %s\\bin\\%s.exe" };
+      String[] templates = new String[] {
+          "%s\\" + chimera + "%s\\bin\\" + chimeraExe + ".exe" };
       String[] roots = new String[] { "\\Program Files",
           "C:\\Program Files", "\\Program Files (x86)",
           "C:\\Program Files (x86)", String.format("%s\\AppData\\Local",
                   System.getProperty("user.home")) };
-      Console.debug("Starting search for " + chimera);
-      for (String root : roots)
+      String versionSeparator = " ";
+      for (File f : FileUtils.getMatchingVersionedFiles(templates, roots,
+              versionsWhitelist, versionsBlacklist, versionSeparator, true))
       {
-        Console.debug("Looking under '" + root + "'");
-        for (String template : templates)
-        {
-          Console.debug("Using template '" + template + "'");
-          String globMatch = String.format(template, root, chimera, "*",
-                  chimeraExe);
-          Console.debug("Looking for installed versions of " + chimera
-                  + " using '" + globMatch + "'");
-          List<File> foundInstalls = FileUtils.getFilesFromGlob(globMatch,
-                  false);
-          for (File found : foundInstalls)
-          {
-            Console.debug("Checking " + found.getPath() + " is okay");
-            boolean add = true;
-            for (String notVersion : versionsBlacklist)
-            {
-              if (String.format(template, root, chimera, notVersion,
-                      chimeraExe).equals(found.getPath()))
-              {
-                add = false;
-                break;
-              }
-            }
-            if (add)
-            {
-              Console.debug("Adding " + found.getPath());
-              installedChimera.add(found);
-            }
-            else
-            {
-              Console.debug(
-                      "Not adding " + found.getPath() + ", not valid");
-            }
-          }
-        }
-        // try without a version number too
-        String nonVersioned = String.format("%s\\%s\\bin\\%s", root,
-                chimera, chimeraExe);
-        installedChimera
-                .addAll(FileUtils.getFilesFromGlob(nonVersioned, false));
-
-        for (File file : installedChimera)
-        {
-          pathList.add(file.getPath());
-        }
-
-        String[] candidates = isChimeraX ? CHIMERAX_VERSIONS
-                : CHIMERA_VERSIONS;
-        for (String version : candidates)
-        {
-          // TODO original code doesn't include version in path; which is right?
-          String path = String.format("%s\\%s %s\\bin\\%s", root, chimera,
-                  version, chimeraExe);
-          pathList.add(path);
-          pathList.add(path + ".exe");
-        }
-        // try without a version number too
-        String path = String.format("%s\\%s\\bin\\%s", root, chimera,
-                chimeraExe);
-        pathList.add(path);
-        pathList.add(path + ".exe");
+        pathList.add(f.getPath());
       }
     }
     else if (Platform.isMac())
     {
       Set<File> installedChimera = new HashSet<>();
       String[] templates = new String[] {
-          "%s/%s-%s.app/Contents/MacOS/%s" };
+          "%s/" + chimera + "%s.app/Contents/MacOS/" + chimeraExe };
       String[] roots = new String[] { "/Applications", String
               .format("%s/Applications", System.getProperty("user.home")) };
-      for (String root : roots)
+      String versionSeparator = "-";
+      for (File f : FileUtils.getMatchingVersionedFiles(templates, roots,
+              versionsWhitelist, versionsBlacklist, versionSeparator, true))
       {
-        for (String template : templates)
-        {
-          String globMatch = String.format(template, root, chimera, "*",
-                  chimeraExe);
-          List<File> foundInstalls = FileUtils.getFilesFromGlob(globMatch,
-                  false);
-          for (File found : foundInstalls)
-          {
-            boolean add = true;
-            for (String notVersion : versionsBlacklist)
-            {
-              if (String.format(template, root, chimera, notVersion,
-                      chimeraExe).equals(found.getPath()))
-              {
-                add = false;
-                break;
-              }
-            }
-            if (add)
-            {
-              installedChimera.add(found);
-            }
-          }
-        }
-        // try without a version number too
-        String nonVersioned = String.format("%s/%s.app/Contents/MacOS/%s",
-                root, chimera, chimeraExe);
-        installedChimera
-                .addAll(FileUtils.getFilesFromGlob(nonVersioned, false));
-
-        for (File file : installedChimera)
-        {
-          pathList.add(file.getPath());
-        }
-
-        // check for installations with version numbers first
-        String[] candidates = isChimeraX ? CHIMERAX_VERSIONS
-                : CHIMERA_VERSIONS;
-        for (String version : candidates)
-        {
-
-          for (String template : templates)
-          {
-            pathList.add(String.format(template, root, chimera, version,
-                    chimeraExe));
-          }
-        }
-        pathList.add(String.format("%s/%s.app/Contents/MacOS/%s", root,
-                chimera, chimeraExe));
+        pathList.add(f.getPath());
       }
     }
     return pathList;

diff --git a/src/jalview/util/FileUtils.java b/src/jalview/util/FileUtils.java
index 5f58903..2c761b3 100644 (file)
--- a/src/jalview/util/FileUtils.java
+++ b/src/jalview/util/FileUtils.java
@@ -390,7 +390,8 @@ public class FileUtils
    */
   public static List<File> getMatchingVersionedFiles(String[] templates,
           String[] roots, String[] versionWhitelist,
-          String[] versionBlacklist, String separator)
+          String[] versionBlacklist, String versionSeparator,
+          boolean exists)
   {
     Set<File> matchingFiles = new HashSet<>();
     if (templates == null)
@@ -410,6 +411,8 @@ public class FileUtils
       for (String root : roots)
       {
         Console.debug("Using root '" + root + "'");
+
+        // Blacklist. Use a file glob for version and see what's there
         if (versionBlacklist != null)
         {
           String globMatch = String.format(template, root, "*");
@@ -423,9 +426,9 @@ public class FileUtils
             for (String notVersion : versionBlacklist)
             {
               StringBuilder vSB = new StringBuilder();
-              if (separator != null)
+              if (versionSeparator != null)
               {
-                vSB.append(separator);
+                vSB.append(versionSeparator);
               }
               vSB.append(notVersion);
               String versionString = vSB.toString();
@@ -446,7 +449,7 @@ public class FileUtils
             }
           }
 
-          if (separator != null)
+          if (versionSeparator != null)
           {
             // try without a version number too
             String nonVersioned = String.format(template, root, "");
@@ -455,6 +458,8 @@ public class FileUtils
           }
         }
 
+        // Whitelist. Just add a path for every whitelisted version (or check it
+        // exists).
         if (versionWhitelist != null)
         {
           Console.debug("Adding " + versionWhitelist.length
@@ -462,23 +467,31 @@ public class FileUtils
           for (String addVersion : versionWhitelist)
           {
             StringBuilder vSB = new StringBuilder();
-            if (separator != null)
+            if (versionSeparator != null)
             {
-              vSB.append(separator);
+              vSB.append(versionSeparator);
             }
             vSB.append(addVersion);
             String versionString = vSB.toString();
             String versionPath = String.format(template, root,
                     versionString);
             Console.debug("Adding whitelist path '" + versionPath + "'");
-            matchingFiles.add(new File(versionPath));
+            File file = new File(versionPath);
+            if (file.exists() || !exists)
+            {
+              matchingFiles.add(file);
+            }
           }
 
-          if (separator != null)
+          if (versionSeparator != null)
           {
             // try without a version number too
             String nonVersioned = String.format(template, root, "");
-            matchingFiles.add(new File(nonVersioned));
+            File file = new File(nonVersioned);
+            if (file.exists() || !exists)
+            {
+              matchingFiles.add(file);
+            }
           }
         }
       }