From: Ben Soares Date: Sun, 30 Apr 2023 23:53:23 +0000 (+0100) Subject: JAL-629 Change behaviour of --open GLOB to increment defaultLinkedId to allow --allfr... X-Git-Tag: Release_2_11_3_0~14^2~76 X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=a7474618cc5b839b6b33803f7e992d26b668244d;p=jalview.git JAL-629 Change behaviour of --open GLOB to increment defaultLinkedId to allow --allframes --output --image --close to act on each input file --- diff --git a/src/jalview/bin/Commands.java b/src/jalview/bin/Commands.java index 402024b..f33202d 100644 --- a/src/jalview/bin/Commands.java +++ b/src/jalview/bin/Commands.java @@ -81,11 +81,11 @@ public class Commands headless = h; boolean theseArgsWereParsed = false; - if (argParser != null && argParser.linkedIds() != null) + if (argParser != null && argParser.getLinkedIds() != null) { - for (String id : argParser.linkedIds()) + for (String id : argParser.getLinkedIds()) { - ArgValuesMap avm = argParser.linkedArgs(id); + ArgValuesMap avm = argParser.getLinkedArgs(id); theseArgsWereParsed = true; if (id == null) { @@ -138,7 +138,7 @@ public class Commands protected boolean processLinked(String id) { boolean theseArgsWereParsed = false; - ArgValuesMap avm = argParser.linkedArgs(id); + ArgValuesMap avm = argParser.getLinkedArgs(id); if (avm == null) return true; @@ -571,7 +571,7 @@ public class Commands protected boolean processImages(String id) { - ArgValuesMap avm = argParser.linkedArgs(id); + ArgValuesMap avm = argParser.getLinkedArgs(id); AlignFrame af = afMap.get(id); if (af == null) diff --git a/src/jalview/bin/argparser/Arg.java b/src/jalview/bin/argparser/Arg.java index 91cfe4d..6c3e6ae 100644 --- a/src/jalview/bin/argparser/Arg.java +++ b/src/jalview/bin/argparser/Arg.java @@ -54,6 +54,12 @@ public enum Arg // user. ALLOWALL, // This Arg can use the '*' linkedId to apply to all known // linkedIds + INCREMENTDEFAULTCOUNTER, // If an Arg has this option and the default + // linkedId is used, the defaultLinkedIdCounter is + // incremented *first*. + INPUT, // This Arg counts as an input for REQUIREINPUT + REQUIREINPUT, // This Arg can only be applied via --allframes if there is an + // input (i.e. --open or --append) } static @@ -81,9 +87,9 @@ public enum Arg SORTBYTREE.setOptions(true, Opt.BOOLEAN); USAGESTATS.setOptions(true, Opt.BOOLEAN); APPEND.setOptions(Opt.STRING, Opt.LINKED, Opt.MULTI, Opt.GLOB, - Opt.ALLOWSUBSTITUTIONS); - OPEN.setOptions(Opt.STRING, Opt.LINKED, Opt.MULTI, Opt.GLOB, - Opt.ALLOWSUBSTITUTIONS); + Opt.ALLOWSUBSTITUTIONS, Opt.INPUT); + OPEN.setOptions(Opt.STRING, Opt.LINKED, Opt.INCREMENTDEFAULTCOUNTER, + Opt.MULTI, Opt.GLOB, Opt.ALLOWSUBSTITUTIONS, Opt.INPUT); PROPS.setOptions(Opt.STRING, Opt.BOOTSTRAP); QUESTIONNAIRE.setOptions(Opt.BOOLEAN, Opt.BOOTSTRAP); SETPROP.setOptions(Opt.STRING, Opt.MULTI, Opt.BOOTSTRAP); @@ -93,7 +99,7 @@ public enum Arg VSESS.setOptions(Opt.UNARY); OUTPUT.setOptions(Opt.STRING, Opt.LINKED, Opt.ALLOWSUBSTITUTIONS, - Opt.ALLOWALL); + Opt.ALLOWALL, Opt.REQUIREINPUT); SSANNOTATIONS.setOptions(Opt.BOOLEAN, Opt.LINKED); NOTEMPFAC.setOptions(Opt.UNARY, Opt.LINKED); @@ -107,7 +113,7 @@ public enum Arg STRUCTUREVIEWER.setOptions(Opt.STRING, Opt.LINKED, Opt.MULTI); WRAP.setOptions(Opt.BOOLEAN, Opt.LINKED); IMAGE.setOptions(Opt.STRING, Opt.LINKED, Opt.ALLOWSUBSTITUTIONS, - Opt.ALLOWALL); + Opt.ALLOWALL, Opt.REQUIREINPUT); QUIT.setOptions(Opt.UNARY); CLOSE.setOptions(Opt.UNARY, Opt.LINKED, Opt.ALLOWALL); DEBUG.setOptions(Opt.BOOLEAN, Opt.BOOTSTRAP); @@ -123,6 +129,7 @@ public enum Arg UNSETARGFILE.setOptions(Opt.MULTI, Opt.PRIVATE, Opt.NOACTION); WEBSERVICEDISCOVERY.setOptions(Opt.BOOLEAN, Opt.BOOTSTRAP); ALLFRAMES.setOptions(Opt.BOOLEAN, Opt.MULTI, Opt.NOACTION); + } private final String[] argNames; diff --git a/src/jalview/bin/argparser/ArgParser.java b/src/jalview/bin/argparser/ArgParser.java index 19e8bea..415e065 100644 --- a/src/jalview/bin/argparser/ArgParser.java +++ b/src/jalview/bin/argparser/ArgParser.java @@ -26,7 +26,9 @@ import java.nio.file.Files; import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.EnumSet; +import java.util.Enumeration; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -107,12 +109,14 @@ public class ArgParser protected Map linkedArgs = new HashMap<>(); - protected List linkedOrder = null; + protected List linkedOrder = new ArrayList<>(); - protected List argList; + protected List argList = new ArrayList<>(); private static final char ARGFILECOMMENT = '#'; + private int argIndex = 0; + static { argMap = new HashMap<>(); @@ -193,19 +197,19 @@ public class ArgParser boolean allowPrivate) { this.substitutions = initsubstitutions; - int argIndex = 0; 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 "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 + // 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("-") && (new File(arg).exists() + && !arg.startsWith("-") && !arg.equals("open") + && (new File(arg).exists() || HttpUtils.startsWithHttpOrHttps(arg))) { arg = Arg.OPEN.argString(); @@ -256,7 +260,9 @@ public class ArgParser if (a == null) { // arg not found - Console.error("Argument '" + arg + "' not recognised. Ignoring."); + Console.error("Argument '" + arg + "' not recognised. Exiting."); + Jalview.exit("Unrecognised command line argument '" + arg + "'", + 13); continue; } if (a.hasOption(Opt.PRIVATE) && !allowPrivate) @@ -367,37 +373,28 @@ public class ArgParser String autoCounterString = null; boolean usingAutoCounterLinkedId = false; - String defaultLinkedId = new StringBuilder(DEFAULTLINKEDIDPREFIX) - .append(Integer.toString(defaultLinkedIdCounter)) - .toString(); + String defaultLinkedId = defaultLinkedId(false); boolean usingDefaultLinkedId = false; if (a.hasOption(Opt.LINKED)) { if (linkedId == null) { - if (a == Arg.OPEN) + if (a.hasOption(Opt.INCREMENTDEFAULTCOUNTER)) { // use the next default prefixed OPENLINKEDID - // NOW using the linkedIdAutoCounter - defaultLinkedIdCounter++; - linkedId = new StringBuilder(OPENLINKEDIDPREFIX) - .append(Integer.toString(defaultLinkedIdCounter)) - .toString(); + defaultLinkedId = defaultLinkedId(true); + } + if (allLinkedIds && a.hasOption(Opt.ALLOWALL)) + { + linkedId = MATCHALLLINKEDIDS; } else { - if (allLinkedIds && a.hasOption(Opt.ALLOWALL)) - { - linkedId = this.MATCHALLLINKEDIDS; - } - else - { - // use default linkedId for linked arguments - linkedId = defaultLinkedId; - usingDefaultLinkedId = true; - Console.debug("Changing linkedId to '" + linkedId - + "' from " + arg); - } + // use default linkedId for linked arguments + linkedId = defaultLinkedId; + usingDefaultLinkedId = true; + Console.debug("Changing linkedId to '" + linkedId + "' from " + + arg); } } else if (linkedId.contains(LINKEDIDAUTOCOUNTER)) @@ -422,13 +419,13 @@ public class ArgParser } } - if (!linkedArgs.containsKey(linkedId)) - linkedArgs.put(linkedId, new ArgValuesMap()); - // do not continue for NOACTION args if (a.hasOption(Opt.NOACTION)) continue; + if (!linkedArgs.containsKey(linkedId)) + linkedArgs.put(linkedId, new ArgValuesMap()); + ArgValuesMap avm = linkedArgs.get(linkedId); // not dealing with both NODUPLICATEVALUES and GLOB @@ -450,7 +447,6 @@ public class ArgParser continue; } - boolean argIndexIncremented = false; /* TODO * Change all avs.addValue() avs.setBoolean avs.setNegated() avs.incrementCount calls to checkfor linkedId == "*" * DONE, need to check @@ -463,11 +459,23 @@ public class ArgParser if (a.hasOption(Opt.GLOB) && globVals != null && globVals.size() > 0) { - for (String v : globVals) + Enumeration gve = Collections.enumeration(globVals); + while (gve.hasMoreElements()) { + String v = gve.nextElement(); SubVals vsv = new SubVals(globSubVals, v); addValue(linkedId, avs, vsv, v, argIndex++, true); - argIndexIncremented = true; + // if we're using defaultLinkedId and the arg increments the + // counter: + if (gve.hasMoreElements() && usingDefaultLinkedId + && a.hasOption(Opt.INCREMENTDEFAULTCOUNTER)) + { + // increment the default linkedId + linkedId = defaultLinkedId(true); + // get new avm and avs + avm = linkedArgs.get(linkedId); + avs = avm.getOrCreateArgValues(a); + } } } else @@ -484,28 +492,53 @@ public class ArgParser { setBoolean(linkedId, avs, true, argIndex); } - incrementCount(linkedId, avs); - if (!argIndexIncremented) - argIndex++; - // store in appropriate place - if (a.hasOption(Opt.LINKED)) + // remove the '*' linkedId that should be empty if it was created + if (MATCHALLLINKEDIDS.equals(linkedId) + && linkedArgs.containsKey(linkedId)) { - // store the order of linkedIds - if (linkedOrder == null) - linkedOrder = new ArrayList<>(); - if (!linkedOrder.contains(linkedId)) - linkedOrder.add(linkedId); + linkedArgs.remove(linkedId); } + } + } + } - // store arg in the list of args used - if (argList == null) - argList = new ArrayList<>(); - if (!argList.contains(a)) - argList.add(a); + private void finaliseStoringArgValue(String linkedId, ArgValues avs) + { + Arg a = avs.arg(); + incrementCount(linkedId, avs); + argIndex++; + + // store in appropriate place + if (a.hasOption(Opt.LINKED)) + { + // store the order of linkedIds + if (!linkedOrder.contains(linkedId)) + linkedOrder.add(linkedId); + } + + // store arg in the list of args used + if (!argList.contains(a)) + argList.add(a); + } + private String defaultLinkedId(boolean increment) + { + String defaultLinkedId = new StringBuilder(DEFAULTLINKEDIDPREFIX) + .append(Integer.toString(defaultLinkedIdCounter)).toString(); + if (increment) + { + while (linkedArgs.containsKey(defaultLinkedId)) + { + defaultLinkedIdCounter++; + defaultLinkedId = new StringBuilder(DEFAULTLINKEDIDPREFIX) + .append(Integer.toString(defaultLinkedIdCounter)) + .toString(); } } + if (!linkedArgs.containsKey(defaultLinkedId)) + linkedArgs.put(defaultLinkedId, new ArgValuesMap()); + return defaultLinkedId; } public String makeSubstitutions(String val, String linkedId) @@ -618,12 +651,12 @@ public class ArgParser return avs == null ? a.getDefaultBoolValue() : avs.getBoolean(); } - public List linkedIds() + public List getLinkedIds() { return linkedOrder; } - public ArgValuesMap linkedArgs(String id) + public ArgValuesMap getLinkedArgs(String id) { return linkedArgs.get(id); } @@ -634,16 +667,16 @@ public class ArgParser StringBuilder sb = new StringBuilder(); sb.append("UNLINKED\n"); sb.append(argValuesMapToString(linkedArgs.get(null))); - if (linkedIds() != null) + if (getLinkedIds() != null) { sb.append("LINKED\n"); - for (String id : linkedIds()) + for (String id : getLinkedIds()) { // already listed these as UNLINKED args if (id == null) continue; - ArgValuesMap avm = linkedArgs(id); + ArgValuesMap avm = getLinkedArgs(id); sb.append("ID: '").append(id).append("'\n"); sb.append(argValuesMapToString(avm)); } @@ -802,17 +835,23 @@ public class ArgParser Arg a = avs.arg(); if (MATCHALLLINKEDIDS.equals(linkedId) && a.hasOption(Opt.ALLOWALL)) { - for (String id : linkedOrder) + for (String id : getLinkedIds()) { + if (id == null || MATCHALLLINKEDIDS.equals(id)) + continue; ArgValuesMap avm = linkedArgs.get(id); - ArgValues tavs = avm.getArgValues(a); + if (a.hasOption(Opt.REQUIREINPUT) + && !avm.hasArgWithOption(Opt.INPUT)) + continue; + ArgValues tavs = avm.getOrCreateArgValues(a); String val = v; if (doSubs) { - val = makeSubstitutions(v, linkedId); + val = makeSubstitutions(v, id); sv = new SubVals(sv, val); } tavs.addValue(sv, val, argIndex); + finaliseStoringArgValue(id, tavs); } } else @@ -824,6 +863,7 @@ public class ArgParser sv = new SubVals(sv, val); } avs.addValue(sv, val, argIndex); + finaliseStoringArgValue(linkedId, avs); } } @@ -833,18 +873,26 @@ public class ArgParser Arg a = avs.arg(); if (MATCHALLLINKEDIDS.equals(linkedId) && a.hasOption(Opt.ALLOWALL)) { - for (String id : linkedOrder) + for (String id : getLinkedIds()) { + if (id == null || MATCHALLLINKEDIDS.equals(id)) + continue; ArgValuesMap avm = linkedArgs.get(id); - ArgValues tavs = avm.getArgValues(a); - String val = doSubs ? makeSubstitutions(v, linkedId) : v; + // don't set an output if there isn't an input + if (a.hasOption(Opt.REQUIREINPUT) + && !avm.hasArgWithOption(Opt.INPUT)) + continue; + ArgValues tavs = avm.getOrCreateArgValues(a); + String val = doSubs ? makeSubstitutions(v, id) : v; tavs.addValue(val, argIndex); + finaliseStoringArgValue(id, tavs); } } else { String val = doSubs ? makeSubstitutions(v, linkedId) : v; avs.addValue(val, argIndex); + finaliseStoringArgValue(linkedId, avs); } } @@ -854,16 +902,23 @@ public class ArgParser Arg a = avs.arg(); if (MATCHALLLINKEDIDS.equals(linkedId) && a.hasOption(Opt.ALLOWALL)) { - for (String id : linkedOrder) + for (String id : getLinkedIds()) { + if (id == null || MATCHALLLINKEDIDS.equals(id)) + continue; ArgValuesMap avm = linkedArgs.get(id); - ArgValues tavs = avm.getArgValues(a); + if (a.hasOption(Opt.REQUIREINPUT) + && !avm.hasArgWithOption(Opt.INPUT)) + continue; + ArgValues tavs = avm.getOrCreateArgValues(a); tavs.setBoolean(b, argIndex); + finaliseStoringArgValue(id, tavs); } } else { avs.setBoolean(b, argIndex); + finaliseStoringArgValue(linkedId, avs); } } @@ -872,10 +927,15 @@ public class ArgParser Arg a = avs.arg(); if (MATCHALLLINKEDIDS.equals(linkedId) && a.hasOption(Opt.ALLOWALL)) { - for (String id : linkedOrder) + for (String id : getLinkedIds()) { + if (id == null || MATCHALLLINKEDIDS.equals(id)) + continue; ArgValuesMap avm = linkedArgs.get(id); - ArgValues tavs = avm.getArgValues(a); + if (a.hasOption(Opt.REQUIREINPUT) + && !avm.hasArgWithOption(Opt.INPUT)) + continue; + ArgValues tavs = avm.getOrCreateArgValues(a); tavs.setNegated(b); } } @@ -890,10 +950,15 @@ public class ArgParser Arg a = avs.arg(); if (MATCHALLLINKEDIDS.equals(linkedId) && a.hasOption(Opt.ALLOWALL)) { - for (String id : linkedOrder) + for (String id : getLinkedIds()) { + if (id == null || MATCHALLLINKEDIDS.equals(id)) + continue; ArgValuesMap avm = linkedArgs.get(id); - ArgValues tavs = avm.getArgValues(a); + if (a.hasOption(Opt.REQUIREINPUT) + && !avm.hasArgWithOption(Opt.INPUT)) + continue; + ArgValues tavs = avm.getOrCreateArgValues(a); tavs.incrementCount(); } } diff --git a/src/jalview/bin/argparser/ArgValuesMap.java b/src/jalview/bin/argparser/ArgValuesMap.java index 63e5302..cbc6faa 100644 --- a/src/jalview/bin/argparser/ArgValuesMap.java +++ b/src/jalview/bin/argparser/ArgValuesMap.java @@ -226,4 +226,17 @@ public class ArgValuesMap return dirname ? FileUtils.getDirname(file) : FileUtils.getBasename(file); } + + /* + * Checks if there is an Arg with Opt + */ + public boolean hasArgWithOption(Opt o) + { + for (Arg a : getArgKeys()) + { + if (a.hasOption(o)) + return true; + } + return false; + } } diff --git a/test/jalview/bin/argparser/ArgParserTest.java b/test/jalview/bin/argparser/ArgParserTest.java index 8d4540a..314b0f0 100644 --- a/test/jalview/bin/argparser/ArgParserTest.java +++ b/test/jalview/bin/argparser/ArgParserTest.java @@ -7,10 +7,12 @@ import java.util.Properties; import org.testng.Assert; import org.testng.annotations.AfterClass; +import org.testng.annotations.AfterMethod; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import jalview.bin.Cache; +import jalview.gui.Desktop; @Test(singleThreaded = true) public class ArgParserTest @@ -21,6 +23,13 @@ public class ArgParserTest Cache.loadProperties("test/jalview/testProps.jvprops"); } + @AfterMethod(alwaysRun = true) + public void tearDown() + { + if (Desktop.instance != null) + Desktop.instance.closeAll_actionPerformed(null); + } + @Test(groups = "Functional", dataProvider = "argLines") public void parseArgsTest(String commandLineArgs, Arg a, String other) { @@ -35,7 +44,7 @@ public class ArgParserTest { String[] args = commandLineArgs.split("\\s+"); ArgParser argparser = new ArgParser(args); - ArgValuesMap avm = argparser.linkedArgs(linkedId); + ArgValuesMap avm = argparser.getLinkedArgs(linkedId); ArgValue av = avm.getArgValue(a); SubVals sv = av.getSubVals(); String testString = null; @@ -67,7 +76,7 @@ public class ArgParserTest // Arg.OPEN, "filename1" }, String[] args = commandLineArgs.split("\\s+"); ArgParser argparser = new ArgParser(args); - ArgValuesMap avm = argparser.linkedArgs(linkedId); + ArgValuesMap avm = argparser.getLinkedArgs(linkedId); ArgValue av = avm.getArgValue(a); Assert.assertEquals(av.getValue(), filename); } @@ -260,23 +269,63 @@ public class ArgParserTest Arg.ARGFILE, "test/jalview/bin/argfiles/testfiles/test1.fa" } }; } - @DataProvider(name = "allLinkedIdData") - public Object[][] allLinkedIdData() + @Test(groups = "Functional", dataProvider = "allLinkedIdsData") + public void allLinkedIdsTest(String commandLineArgs, Arg a, + String[] values) + { + String[] args = commandLineArgs.split("\\s+"); + ArgParser argparser = new ArgParser(args); + + int num = values.length; + List linkedIds = argparser.getLinkedIds(); + Assert.assertEquals(linkedIds.size(), num, + "Wrong number of linkedIds: " + linkedIds.toString()); + for (int i = 0; i < num; i++) + { + String value = values[i]; + String linkedId = linkedIds.get(i); + ArgValuesMap avm = argparser.getLinkedArgs(linkedId); + if (value == null) + { + Assert.assertTrue(avm.containsArg(a), + "Arg value for " + a.argString() + + " not applied correctly to linkedId '" + linkedId + + "'"); + } + else + { + ArgValues avs = avm.getArgValues(a); + ArgValue av = avs.getArgValue(); + String v = av.getValue(); + value = new File(value).getAbsolutePath(); + Assert.assertEquals(v, value, "Arg value for " + a.argString() + + " not applied correctly to linkedId '" + linkedId + "'"); + } + } + + } + + @DataProvider(name = "allLinkedIdsData") + public Object[][] allLinkedIdsData() { return new Object[][] { // - { "--open=test/jalview/bin/argparser/testfiles/*.fa --substitutions --all --output={dirname}/{basename}.stk --close", + /* + */ + { "--open=test/jalview/bin/argparser/testfiles/*.fa --substitutions --allframes --image={dirname}/{basename}.png --close", + Arg.CLOSE, new String[] + { null, null, + null } }, + { "--open=test/jalview/bin/argparser/testfiles/*.fa --substitutions --allframes --output={dirname}/{basename}.stk --close", Arg.OUTPUT, new String[] - { "test/jalview/bin/argfiles/testfiles/test1.stk", - "test/jalview/bin/argfiles/testfiles/test2.stk", - "test/jalview/bin/argfiles/testfiles/test3.stk", } }, - { "--open=test/jalview/bin/argparser/testfiles/*.fa --substitutions --all --image={dirname}/{basename}.png --close", + { "test/jalview/bin/argparser/testfiles/test1.stk", + "test/jalview/bin/argparser/testfiles/test2.stk", + "test/jalview/bin/argparser/testfiles/test3.stk", } }, + { "--open=test/jalview/bin/argparser/testfiles/*.fa --substitutions --allframes --image={dirname}/{basename}.png --close", Arg.IMAGE, new String[] - { "test/jalview/bin/argfiles/testfiles/test1.png", - "test/jalview/bin/argfiles/testfiles/test2.png", - "test/jalview/bin/argfiles/testfiles/test3.png", } }, - { "--open=test/jalview/bin/argparser/testfiles/*.fa --substitutions --all --image={dirname}/{basename}.png --close", - Arg.CLOSE, null }, + { "test/jalview/bin/argparser/testfiles/test1.png", + "test/jalview/bin/argparser/testfiles/test2.png", + "test/jalview/bin/argparser/testfiles/test3.png", } }, // }; }