From: Ben Soares <b.soares@dundee.ac.uk>
Date: Mon, 11 Sep 2023 23:21:10 +0000 (+0100)
Subject: Merge branch 'improvement/JAL-4262_deprecation_warning_when_old_CLI_arguments_are_pro... 
X-Git-Tag: Release_2_11_3_0~8^2~5^2~30
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=0798a7c77a9f2193cc7179215d28197c7a443e16;hp=-c;p=jalview.git

Merge branch 'improvement/JAL-4262_deprecation_warning_when_old_CLI_arguments_are_processed' into bug/JAL-4277_dirname_substitution_always_absolute
---

0798a7c77a9f2193cc7179215d28197c7a443e16
diff --combined src/jalview/bin/Commands.java
index cd72e60,d910819..e6c7b54
--- a/src/jalview/bin/Commands.java
+++ b/src/jalview/bin/Commands.java
@@@ -14,6 -14,7 +14,7 @@@ import java.util.Map
  
  import jalview.analysis.AlignmentUtils;
  import jalview.api.structures.JalviewStructureDisplayI;
+ import jalview.bin.Jalview.ExitCode;
  import jalview.bin.argparser.Arg;
  import jalview.bin.argparser.ArgParser;
  import jalview.bin.argparser.ArgParser.Position;
@@@ -72,8 -73,6 +73,8 @@@ public class Command
  
    private boolean argsWereParsed = false;
  
 +  private List<String> errors = new ArrayList<>();
 +
    public Commands(ArgParser argparser, boolean headless)
    {
      this(Desktop.instance, argparser, headless);
@@@ -140,28 -139,11 +141,30 @@@
        }
  
      }
 +
 +    // report errors
 +    StringBuilder sb = new StringBuilder();
 +    for (String error : errors)
 +    {
 +      sb.append("- " + error);
 +      sb.append("\n");
 +    }
 +    if (Platform.isHeadless())
 +    {
 +      Console.debug("All errors from command line argument commands:\n"
 +              + sb.toString());
 +    }
 +    else
 +    {
 +      // scrollable dialog box
 +
 +    }
 +
      if (argParser.getBoolean(Arg.QUIT))
      {
-       Jalview.exit("Exiting due to " + Arg.QUIT.argString(), 0);
+       Jalview.getInstance().exit(
+               "Exiting due to " + Arg.QUIT.argString() + " argument.",
+               ExitCode.OK);
        return true;
      }
      // carry on with jalview.bin.Jalview
@@@ -184,11 -166,7 +187,11 @@@
      boolean theseArgsWereParsed = false;
      ArgValuesMap avm = argParser.getLinkedArgs(id);
      if (avm == null)
 +    {
        return true;
 +    }
 +
 +    boolean isError = false;
  
      // set wrap scope here so it can be applied after structures are opened
      boolean wrap = false;
@@@ -241,8 -219,7 +244,8 @@@
            {
              if (!(new File(openFile)).exists())
              {
 -              Console.warn("Can't find file '" + openFile + "'");
 +              addError("Can't find file '" + openFile + "'");
 +              isError = true;
                continue;
              }
            }
@@@ -257,8 -234,7 +260,8 @@@
            format = new IdentifyFile().identify(openFile, protocol);
          } catch (FileFormatException e1)
          {
 -          Console.error("Unknown file format for '" + openFile + "'");
 +          addError("Unknown file format for '" + openFile + "'");
 +          isError = true;
            continue;
          }
  
@@@ -284,16 -260,15 +287,16 @@@
            } catch (Throwable thr)
            {
              xception = true;
 -            Console.error("Couldn't open '" + openFile + "' as " + format
 -                    + " " + thr.getLocalizedMessage()
 +            addError("Couldn't open '" + openFile + "' as " + format + " "
 +                    + thr.getLocalizedMessage()
                      + " (Enable debug for full stack trace)");
 +            isError = true;
              Console.debug("Exception when opening '" + openFile + "'", thr);
            } finally
            {
              if (af == null && !xception)
              {
 -              Console.info("Ignoring '" + openFile
 +              addInfo("Ignoring '" + openFile
                        + "' - no alignment data found.");
                continue;
              }
@@@ -310,7 -285,8 +313,7 @@@
  
              if (cs == null && !"None".equals(colour))
              {
 -              Console.warn(
 -                      "Couldn't parse '" + colour + "' as a colourscheme.");
 +              addWarn("Couldn't parse '" + colour + "' as a colourscheme.");
              }
              else
              {
@@@ -374,8 -350,7 +377,8 @@@
                        "examples/testdata/uniref50_test_tree", treefile);
              } catch (IOException e)
              {
 -              Console.warn("Couldn't add tree " + treefile, e);
 +              addError("Couldn't add tree " + treefile, e);
 +              isError = true;
              }
            }
  
@@@ -441,11 -416,12 +444,12 @@@
        {
          if (headless)
          {
-           Jalview.exit("Could not open any files in headless mode", 1);
+           Jalview.exit("Could not open any files in headless mode",
+                   ExitCode.NO_FILES);
          }
          else
          {
 -          Console.warn("No more files to open");
 +          Console.info("No more files to open");
          }
        }
        if (progressBarSet && desktop != null)
@@@ -475,8 -451,10 +479,8 @@@
  
            if (seq == null)
            {
 -            Console.warn("Could not find sequence for argument "
 +            addWarn("Could not find sequence for argument "
                      + Arg.STRUCTURE.argString() + "=" + val);
 -            // you probably want to continue here, not break
 -            // break;
              continue;
            }
            File structureFile = null;
@@@ -509,14 -487,14 +513,14 @@@
  
            if (structureFile == null)
            {
 -            Console.warn("Not provided structure file with '" + val + "'");
 +            addWarn("Not provided structure file with '" + val + "'");
              continue;
            }
  
            if (!structureFile.exists())
            {
 -            Console.warn("Structure file '"
 -                    + structureFile.getAbsoluteFile() + "' not found.");
 +            addWarn("Structure file '" + structureFile.getAbsoluteFile()
 +                    + "' not found.");
              continue;
            }
  
@@@ -548,7 -526,7 +552,7 @@@
              } catch (IOException e)
              {
                paeFilepath = paeFile.getAbsolutePath();
 -              Console.warn("Problem with the PAE file path: '"
 +              addWarn("Problem with the PAE file path: '"
                        + paeFile.getPath() + "'");
              }
            }
@@@ -591,7 -569,7 +595,7 @@@
                  if (it.hasNext())
                    sb.append(", ");
                }
 -              Console.warn(sb.toString());
 +              addWarn(sb.toString());
              }
            }
  
@@@ -608,8 -586,7 +612,8 @@@
  
            if (sv == null)
            {
 -            Console.error("Failed to import and open structure view.");
 +            addError("Failed to import and open structure view for file '"
 +                    + structureFile + "'.");
              continue;
            }
            try
@@@ -627,15 -604,13 +631,15 @@@
              }
              if (tries == 0 && sv.isBusy())
              {
 -              Console.warn(
 -                      "Gave up waiting for structure viewer to load. Something may have gone wrong.");
 +              addWarn("Gave up waiting for structure viewer to load file '"
 +                      + structureFile
 +                      + "'. Something may have gone wrong.");
              }
            } catch (Exception x)
            {
 -            Console.warn("Exception whilst waiting for structure viewer "
 +            addError("Exception whilst waiting for structure viewer "
                      + structureFilepath, x);
 +            isError = true;
            }
  
            // add StructureViewer to svMap list
@@@ -684,7 -659,7 +688,7 @@@
                        typeS.toUpperCase(Locale.ROOT));
              } catch (IllegalArgumentException e)
              {
 -              Console.warn("Do not know image format '" + typeS
 +              addWarn("Do not know image format '" + typeS
                        + "', using PNG");
                imageType = TYPE.PNG;
              }
@@@ -701,18 -676,6 +705,18 @@@
                  AppJmol jmol = (AppJmol) sview;
                  try
                  {
 +                  whatNext wn = this.checksBeforeWritingToFile(avm, subVals,
 +                          false, structureImageFilename, "structure image");
 +                  if (wn == whatNext.ERROR)
 +                  {
 +                    isError = true;
 +                    continue;
 +                  }
 +                  else if (wn == whatNext.CONTINUE)
 +                  {
 +                    continue;
 +                  }
 +
                    Console.debug("Rendering image to " + structureImageFile);
                    jmol.makePDBImage(structureImageFile, imageType, renderer,
                            userBis);
@@@ -721,18 -684,16 +725,18 @@@
  
                  } catch (ImageOutputException ioexc)
                  {
 -                  Console.warn("Unexpected error whilst exporting image to "
 +                  addError("Unexpected error whilst exporting image to "
                            + structureImageFile, ioexc);
 +                  isError = true;
 +                  continue;
                  }
  
                }
                break;
              default:
 -              Console.warn("Cannot export image for structure viewer "
 +              addWarn("Cannot export image for structure viewer "
                        + sv.getViewerType() + " yet");
 -              break;
 +              continue;
              }
            }
          }
@@@ -765,7 -726,7 +769,7 @@@
      }
      */
  
 -    return theseArgsWereParsed;
 +    return theseArgsWereParsed && !isError;
    }
  
    protected void processGroovyScript(String id)
@@@ -775,7 -736,7 +779,7 @@@
  
      if (af == null)
      {
 -      Console.warn("Did not have an alignment window for id=" + id);
 +      addWarn("Did not have an alignment window for id=" + id);
        return;
      }
  
@@@ -799,11 -760,10 +803,11 @@@
  
      if (af == null)
      {
 -      Console.warn("Did not have an alignment window for id=" + id);
 +      addWarn("Did not have an alignment window for id=" + id);
        return false;
      }
  
 +    boolean isError = false;
      if (avm.containsArg(Arg.IMAGE))
      {
        for (ArgValue av : avm.getArgValueList(Arg.IMAGE))
@@@ -844,19 -804,6 +848,19 @@@
          Cache.setProperty("EXPORT_EMBBED_BIOJSON", "false");
  
          Console.info("Writing " + file);
 +
 +        whatNext wn = this.checksBeforeWritingToFile(avm, subVal, false,
 +                fileName, "image");
 +        if (wn == whatNext.ERROR)
 +        {
 +          isError = true;
 +          continue;
 +        }
 +        else if (wn == whatNext.CONTINUE)
 +        {
 +          continue;
 +        }
 +
          try
          {
            switch (type)
@@@ -904,19 -851,17 +908,19 @@@
              break;
  
            default:
 -            Console.warn(Arg.IMAGE.argString() + " type '" + type
 +            addWarn(Arg.IMAGE.argString() + " type '" + type
                      + "' not known. Ignoring");
              break;
            }
          } catch (Exception ioex)
          {
 -          Console.warn("Unexpected error during export", ioex);
 +          addError("Unexpected error during export to '" + fileName + "'",
 +                  ioex);
 +          isError = true;
          }
        }
      }
 -    return true;
 +    return !isError;
    }
  
    protected boolean processOutput(String id)
@@@ -926,12 -871,10 +930,12 @@@
  
      if (af == null)
      {
 -      Console.warn("Did not have an alignment window for id=" + id);
 +      addWarn("Did not have an alignment window for id=" + id);
        return false;
      }
  
 +    boolean isError = false;
 +
      if (avm.containsArg(Arg.OUTPUT))
      {
        for (ArgValue av : avm.getArgValueList(Arg.OUTPUT))
@@@ -941,6 -884,24 +945,6 @@@
          String fileName = subVals.getContent();
          boolean stdout = ArgParser.STDOUTFILENAME.equals(fileName);
          File file = new File(fileName);
 -        boolean overwrite = ArgParser.getFromSubValArgOrPref(avm,
 -                Arg.OVERWRITE, subVals, null, "OVERWRITE_OUTPUT", false);
 -        // backups. Use the Arg.BACKUPS or subval "backups" setting first,
 -        // otherwise if headless assume false, if not headless use the user
 -        // preference with default true.
 -        boolean backups = ArgParser.getFromSubValArgOrPref(avm, Arg.BACKUPS,
 -                subVals, null,
 -                Platform.isHeadless() ? null : BackupFiles.ENABLED,
 -                !Platform.isHeadless());
 -
 -        // if backups is not true then --overwrite must be specified
 -        if (file.exists() && !(overwrite || backups || stdout))
 -        {
 -          Console.error("Won't overwrite file '" + fileName + "' without "
 -                  + Arg.OVERWRITE.argString() + " or "
 -                  + Arg.BACKUPS.argString() + " set");
 -          return false;
 -        }
  
          String name = af.getName();
          String format = ArgParser.getValueFromSubValOrArg(avm, av,
@@@ -990,49 -951,41 +994,49 @@@
                validSB.append(")");
              }
  
 -            Jalview.exit("No valid format specified for "
 +            addError("No valid format specified for "
                      + Arg.OUTPUT.argString() + ". Valid formats are "
 -                    + validSB.toString() + ".", ExitCode.INVALID_FORMAT);
 -            // this return really shouldn't happen
 -            return false;
 +                    + validSB.toString() + ".");
 +            continue;
            }
          }
  
 -        String savedBackupsPreference = Cache
 -                .getDefault(BackupFiles.ENABLED, null);
 -        Console.debug("Setting backups to " + backups);
 -        Cache.applicationProperties.put(BackupFiles.ENABLED,
 -                Boolean.toString(backups));
 +        whatNext wn = this.checksBeforeWritingToFile(avm, subVals, true,
 +                fileName, ff.getName());
 +        if (wn == whatNext.ERROR)
 +        {
 +          isError = true;
 +          continue;
 +        }
 +        else if (wn == whatNext.CONTINUE)
 +        {
 +          continue;
 +        }
 +
 +        boolean backups = ArgParser.getFromSubValArgOrPref(avm, Arg.BACKUPS,
 +                subVals, null,
 +                Platform.isHeadless() ? null : BackupFiles.ENABLED,
 +                !Platform.isHeadless());
  
          Console.info("Writing " + fileName);
  
 -        af.saveAlignment(fileName, ff, stdout);
 -        Console.debug("Returning backups to " + savedBackupsPreference);
 -        if (savedBackupsPreference != null)
 -          Cache.applicationProperties.put(BackupFiles.ENABLED,
 -                  savedBackupsPreference);
 +        af.saveAlignment(fileName, ff, stdout, backups);
          if (af.isSaveAlignmentSuccessful())
          {
            Console.debug("Written alignment '" + name + "' in "
 -                  + ff.getName() + " format to " + file);
 +                  + ff.getName() + " format to '" + file + "'");
          }
          else
          {
 -          Console.warn("Error writing file " + file + " in " + ff.getName()
 +          addError("Error writing file '" + file + "' in " + ff.getName()
                    + " format!");
 +          isError = true;
 +          continue;
          }
  
        }
      }
 -    return true;
 +    return !isError;
    }
  
    private SequenceI getSpecifiedSequence(AlignFrame af, ArgValuesMap avm,
@@@ -1098,78 -1051,4 +1102,78 @@@
      }
      return svs;
    }
 +
 +  private void addInfo(String errorMessage)
 +  {
 +    Console.info(errorMessage);
 +    errors.add(errorMessage);
 +  }
 +
 +  private void addWarn(String errorMessage)
 +  {
 +    Console.warn(errorMessage);
 +    errors.add(errorMessage);
 +  }
 +
 +  private void addError(String errorMessage)
 +  {
 +    addError(errorMessage, null);
 +  }
 +
 +  private void addError(String errorMessage, Exception e)
 +  {
 +    Console.error(errorMessage, e);
 +    errors.add(errorMessage);
 +  }
 +
 +  private enum whatNext
 +  {
 +    OKAY, CONTINUE, ERROR;
 +  }
 +
 +  private whatNext checksBeforeWritingToFile(ArgValuesMap avm,
 +          SubVals subVal, boolean includeBackups, String filename,
 +          String adjective)
 +  {
 +    File file = new File(filename);
 +
 +    boolean overwrite = ArgParser.getFromSubValArgOrPref(avm, Arg.OVERWRITE,
 +            subVal, null, "OVERWRITE_OUTPUT", false);
 +    boolean stdout = false;
 +    boolean backups = false;
 +    if (includeBackups)
 +    {
 +      stdout = ArgParser.STDOUTFILENAME.equals(filename);
 +      // backups. Use the Arg.BACKUPS or subval "backups" setting first,
 +      // otherwise if headless assume false, if not headless use the user
 +      // preference with default true.
 +      backups = ArgParser.getFromSubValArgOrPref(avm, Arg.BACKUPS, subVal,
 +              null, Platform.isHeadless() ? null : BackupFiles.ENABLED,
 +              !Platform.isHeadless());
 +    }
 +
 +    if (file.exists() && !(overwrite || backups || stdout))
 +    {
 +      addWarn("Won't overwrite file '" + filename + "' without "
 +              + Arg.OVERWRITE.argString()
 +              + (includeBackups ? " or " + Arg.BACKUPS.argString() : "")
 +              + " set");
 +      return whatNext.CONTINUE;
 +    }
 +
 +    boolean mkdirs = ArgParser.getFromSubValArgOrPref(avm, Arg.MKDIRS,
 +            subVal, null, "MKDIRS_OUTPUT", false);
 +
 +    if (!FileUtils.checkParentDir(file, mkdirs))
 +    {
 +      addError("Directory '"
 +              + FileUtils.getParentDir(file).getAbsolutePath()
 +              + "' does not exist for " + adjective + " file '" + filename
 +              + "'."
 +              + (mkdirs ? "" : "  Try using " + Arg.MKDIRS.argString()));
 +      return whatNext.ERROR;
 +    }
 +
 +    return whatNext.OKAY;
 +  }
  }