From c24ec44edbedba55005373c43bf712c047b56faa Mon Sep 17 00:00:00 2001 From: Ben Soares Date: Fri, 11 Aug 2023 14:44:07 +0100 Subject: [PATCH] JAL-629 Much better way of dealing with list of filenames/URLs without an --open argument. Added tests for these and fixed multiple STDOUT files. --- src/jalview/bin/argparser/ArgParser.java | 59 ++++++++++++++----------- src/jalview/gui/AlignFrame.java | 18 +++++--- test/jalview/bin/CommandLineOperationsNG.java | 22 +++++++-- test/jalview/bin/test1-3.fa | 6 +++ 4 files changed, 69 insertions(+), 36 deletions(-) create mode 100644 test/jalview/bin/test1-3.fa diff --git a/src/jalview/bin/argparser/ArgParser.java b/src/jalview/bin/argparser/ArgParser.java index 9c66f32..3862375 100644 --- a/src/jalview/bin/argparser/ArgParser.java +++ b/src/jalview/bin/argparser/ArgParser.java @@ -211,12 +211,15 @@ public class ArgParser public ArgParser(String[] args, boolean initsubstitutions, BootstrapArgs bsa) { - // Make a mutable new ArrayList so that shell globbing parser works. - // (When shell file globbing is used, there are a sequence of non-Arg - // arguments (which are the expanded globbed filenames) that need to be - // consumed by the --append/--argfile/etc Arg which is most easily done by - // removing these filenames from the list one at a time. This can't be done - // with an ArrayList made with only Arrays.asList(String[] args). ) + /* + * Make a mutable new ArrayList so that shell globbing parser works. + * (When shell file globbing is used, there are a sequence of non-Arg + * arguments (which are the expanded globbed filenames) that need to be + * consumed by the --append/--argfile/etc Arg which is most easily done + * by removing these filenames from the list one at a time. This can't be + * done with an ArrayList made with only Arrays.asList(String[] args) as + * that is not mutable. ) + */ this(new ArrayList<>(Arrays.asList(args)), initsubstitutions, false, bsa); } @@ -262,27 +265,30 @@ public class ArgParser boolean allowPrivate) { this.substitutions = initsubstitutions; - boolean openEachInitialFilenames = true; - for (int i = 0; i < args.size(); i++) - { - String arg = args.get(i); - // If the first arguments do not start with "--" or "-" or is not "open" - // and` is a filename that exists it is probably a file/list of files to - // open so we fake an Arg.OPEN argument and when adding files only add the - // single arg[i] and increment the defaultLinkedIdCounter so that each of - // these files is opened separately. - if (openEachInitialFilenames && !arg.startsWith(DOUBLEDASH) - && !arg.startsWith("-") && !arg.equals("open") - && (new File(arg).exists() - || HttpUtils.startsWithHttpOrHttps(arg))) - { - arg = Arg.OPEN.argString(); - } - else + /* + * If the first argument does not start with "--" or "-" or is not "open", + * and is a filename that exists or a URL, it is probably a file/list of + * files to open so we insert an Arg.OPEN argument before it. This will + * mean the list of files at the start of the arguments are all opened + * separately. + */ + if (args.size() > 0) + { + String arg0 = args.get(0); + if (arg0 != null + && (!arg0.startsWith(DOUBLEDASH) && !arg0.startsWith("-") + && !arg0.equals("open") && (new File(arg0).exists() + || HttpUtils.startsWithHttpOrHttps(arg0)))) { - openEachInitialFilenames = false; + // insert "--open" at the start + args.add(0, Arg.OPEN.argString()); } + } + + for (int i = 0; i < args.size(); i++) + { + String arg = args.get(i); // look for double-dash, e.g. --arg if (arg.startsWith(DOUBLEDASH)) @@ -433,7 +439,7 @@ public class ArgParser { // There is no "=" so value is next arg or args (possibly shell // glob-expanded) - if ((openEachInitialFilenames ? i : i + 1) >= args.size()) + if (i + 1 >= args.size()) { // no value to take for arg, which wants a value Console.error("Argument '" + a.getName() @@ -448,8 +454,7 @@ public class ArgParser { // if this is the first argument with a file list at the start of // the args we add filenames from index i instead of i+1 - globVals = getShellGlobbedFilenameValues(a, args, - openEachInitialFilenames ? i : i + 1); + globVals = getShellGlobbedFilenameValues(a, args, i + 1); } else { diff --git a/src/jalview/gui/AlignFrame.java b/src/jalview/gui/AlignFrame.java index 99881e6..eb98d98 100644 --- a/src/jalview/gui/AlignFrame.java +++ b/src/jalview/gui/AlignFrame.java @@ -830,7 +830,8 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener, @Override public void propertyChange(PropertyChangeEvent evt) { - // // jalview.bin.Console.outPrintln("Discoverer property change."); + // // jalview.bin.Console.outPrintln("Discoverer property + // change."); // if (evt.getPropertyName().equals("services")) { SwingUtilities.invokeLater(new Runnable() @@ -1331,9 +1332,12 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener, out.print(output); out.flush(); - Console.trace("ALIGNFRAME about to close file"); - out.close(); - Console.trace("ALIGNFRAME closed file"); + if (!stdout) + { + Console.trace("ALIGNFRAME about to close file"); + out.close(); + Console.trace("ALIGNFRAME closed file"); + } AlignFrame.this.setTitle(stdout ? "STDOUT" : file); if (stdout) { @@ -4343,7 +4347,8 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener, { try { - jalview.bin.Console.errPrintln("Waiting for building menu to finish."); + jalview.bin.Console + .errPrintln("Waiting for building menu to finish."); Thread.sleep(10); } catch (Exception e) { @@ -5961,7 +5966,8 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener, } else { - jalview.bin.Console.errPrintln("Can't run Groovy script as console not found"); + jalview.bin.Console + .errPrintln("Can't run Groovy script as console not found"); } } diff --git a/test/jalview/bin/CommandLineOperationsNG.java b/test/jalview/bin/CommandLineOperationsNG.java index ef2f78a..56d4300 100644 --- a/test/jalview/bin/CommandLineOperationsNG.java +++ b/test/jalview/bin/CommandLineOperationsNG.java @@ -162,6 +162,12 @@ public class CommandLineOperationsNG private Worker getJalviewDesktopRunner(boolean withAwt, String cmd, int timeout) { + return getJalviewDesktopRunner(withAwt, cmd, timeout, true); + } + + private Worker getJalviewDesktopRunner(boolean withAwt, String cmd, + int timeout, boolean testoutput) + { /* boolean win = System.getProperty("os.name").indexOf("Win") >= 0; String pwd = ""; @@ -194,7 +200,7 @@ public class CommandLineOperationsNG Worker worker = null; try { - cmd = " --testoutput " + cmd; + cmd = cmd + (testoutput ? " --testoutput " : ""); System.out.println("Running '" + _cmd + cmd + "'"); ls2_proc = Runtime.getRuntime().exec(_cmd + cmd); } catch (Throwable e1) @@ -378,7 +384,7 @@ public class CommandLineOperationsNG { String cmd = args; File file = new File(comparisonFile); - Worker worker = getJalviewDesktopRunner(true, cmd, timeout); + Worker worker = getJalviewDesktopRunner(true, cmd, timeout, false); int b = -1; StringBuilder sb = new StringBuilder(); try @@ -504,7 +510,7 @@ public class CommandLineOperationsNG // when running tests on build server, but we will keep this patch for now // since it works. // https://issues.jalview.org/browse/JAL-1889?focusedCommentId=21609&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-21609 - String workingDir = "test/jalview/bin/"; + String workingDir = "test/jalview/bin"; return new Object[][] { // { "--open=examples/uniref50.fa --output=-", @@ -515,6 +521,16 @@ public class CommandLineOperationsNG workingDir + "/uniref50-output.blc", TEST_TIMEOUT }, { "--open examples/uniref50.fa --output - --format blc", workingDir + "/uniref50-output.blc", TEST_TIMEOUT }, + { "./examples/uniref50.fa --output=-", + workingDir + "/uniref50-output.fa", TEST_TIMEOUT }, + { "./examples/uniref50.fa --output - --format blc", + workingDir + "/uniref50-output.blc", TEST_TIMEOUT }, + // remember you can't use shell wildcards for filenames in a test + { "./test/jalview/bin/argparser/testfiles/test1.fa ./test/jalview/bin/argparser/testfiles/test2.fa ./test/jalview/bin/argparser/testfiles/test3.fa --all --output -", + workingDir + "/test1-3.fa", TEST_TIMEOUT }, + // but you can use java wildcards when using an equals sign + { "--open=./test/jalview/bin/argparser/testfiles/test*.fa --all --output -", + workingDir + "/test1-3.fa", TEST_TIMEOUT }, // }; } diff --git a/test/jalview/bin/test1-3.fa b/test/jalview/bin/test1-3.fa new file mode 100644 index 0000000..c01e6cf --- /dev/null +++ b/test/jalview/bin/test1-3.fa @@ -0,0 +1,6 @@ +>TEST1/1-4 +AAAA +>TEST2/1-4 +LLLL +>TEST3/1-5 +AAARG -- 1.7.10.2