diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 902e01c16..0a052292c 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -18,6 +18,7 @@ Picocli follows [semantic versioning](http://semver.org/). ## Fixed issues +* [#1162] Bugfix: Abbreviated options are not matched if value attached with '=' separator (like `-x=3`). Thanks to [Chris Laprun](https://github.com/metacosm) for raising this. * [#1158] DOC: Fix broken links to GraalVM repo. Thanks to [Andreas Deininger](https://github.com/deining) for the pull request. * [#1155] DOC: Fix sample code in chapter "Validation". Thanks to [Andreas Deininger](https://github.com/deining) for the pull request. * [#1157] DOC: Fix typo "a argument group" in user manual. Thanks to sabrina for raising this. diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java index c2c9aaf3b..8672fa02e 100644 --- a/src/main/java/picocli/CommandLine.java +++ b/src/main/java/picocli/CommandLine.java @@ -12526,11 +12526,7 @@ private void processArguments(List parsedCommands, // if we find another command, we are done with the current command if (commandSpec.parser().abbreviatedSubcommandsAllowed()) { - try { - arg = AbbreviationMatcher.match(commandSpec.subcommands().keySet(), arg, commandSpec.subcommandsCaseInsensitive()); - } catch (IllegalArgumentException ex) { - throw new ParameterException(CommandLine.this, "Error: " + ex.getMessage()); - } + arg = AbbreviationMatcher.match(commandSpec.subcommands().keySet(), arg, commandSpec.subcommandsCaseInsensitive(), CommandLine.this); } if (commandSpec.subcommands().containsKey(arg)) { CommandLine subcommand = commandSpec.subcommands().get(arg); @@ -12553,21 +12549,17 @@ private void processArguments(List parsedCommands, // A single option may be without option parameters, like "-v" or "--verbose" (a boolean value), // or an option may have one or more option parameters. // A parameter may be attached to the option. + Set aggregatedOptionNames = new LinkedHashSet(); if (commandSpec.parser().abbreviatedOptionsAllowed()) { - Set aggregatedOptionNames = new LinkedHashSet(); aggregatedOptionNames.addAll(commandSpec.optionsMap().keySet()); aggregatedOptionNames.addAll(commandSpec.negatedOptionsMap().keySet()); - Iterator iterator = aggregatedOptionNames.iterator(); - try { - arg = AbbreviationMatcher.match(aggregatedOptionNames, arg, commandSpec.optionsCaseInsensitive()); - } catch (IllegalArgumentException ex) { - throw new ParameterException(CommandLine.this, "Error: " + ex.getMessage()); - } + arg = AbbreviationMatcher.match(aggregatedOptionNames, arg, commandSpec.optionsCaseInsensitive(), CommandLine.this); } LookBehind lookBehind = LookBehind.SEPARATE; int separatorIndex = arg.indexOf(separator); if (separatorIndex > 0) { String key = arg.substring(0, separatorIndex); + key = AbbreviationMatcher.match(aggregatedOptionNames, key, commandSpec.optionsCaseInsensitive(), CommandLine.this); //#1159, #1162 // be greedy. Consume the whole arg as an option if possible. if (isStandaloneOption(key) && isStandaloneOption(arg)) { tracer.warn("Both '%s' and '%s' are valid option names in %s. Using '%s'...%n", arg, key, getCommandName(), arg); @@ -17509,8 +17501,9 @@ private static String makeCanonical(String str) { return str; } - public static String match(Set set, String abbreviation, boolean caseInsensitive) { - if (set.contains(abbreviation)) { // return exact match + /** Returns the non-abbreviated name if found, otherwise returns the specified original abbreviation value. */ + public static String match(Set set, String abbreviation, boolean caseInsensitive, CommandLine source) { + if (set.contains(abbreviation) || set.isEmpty()) { // return exact match return abbreviation; } List abbreviatedKeyChunks = splitIntoChunks(abbreviation, caseInsensitive); @@ -17523,7 +17516,7 @@ public static String match(Set set, String abbreviation, boolean caseIns } if (candidates.size() > 1) { String str = candidates.toString(); - throw new IllegalArgumentException("'" + abbreviation + "' is not unique: it matches '" + + throw new ParameterException(source, "Error: '" + abbreviation + "' is not unique: it matches '" + str.substring(1, str.length() - 1).replace(", ", "', '") + "'"); } return candidates.isEmpty() ? abbreviation : candidates.get(0); // return the original if no match found diff --git a/src/test/java/picocli/AbbreviationMatcherTest.java b/src/test/java/picocli/AbbreviationMatcherTest.java index 4185d4e4a..50d6770c6 100644 --- a/src/test/java/picocli/AbbreviationMatcherTest.java +++ b/src/test/java/picocli/AbbreviationMatcherTest.java @@ -1,6 +1,5 @@ package picocli; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.contrib.java.lang.system.ProvideSystemProperty; @@ -10,11 +9,19 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; import java.io.PrintWriter; -import java.util.*; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.Set; import static org.junit.Assert.*; -import static picocli.CommandLine.*; -import static picocli.CommandLine.AbbreviationMatcher.*; +import static picocli.CommandLine.AbbreviationMatcher.match; +import static picocli.CommandLine.AbbreviationMatcher.splitIntoChunks; +import static picocli.CommandLine.Command; +import static picocli.CommandLine.Model; +import static picocli.CommandLine.Option; +import static picocli.CommandLine.ParameterException; +import static picocli.CommandLine.ParseResult; +import static picocli.CommandLine.UnmatchedArgumentException; public class AbbreviationMatcherTest { @@ -42,58 +49,59 @@ private Set createSet() { @Test public void testPrefixMatch() { Set set = createSet(); - - assertEquals("kebab-case", match(set, "kebab-case", false)); - assertEquals("kebab-case-extra", match(set, "kebab-case-extra", false)); - assertEquals("very-long-kebab-case", match(set, "very-long-kebab-case", false)); - assertEquals("very-long-kebab-case", match(set, "v-l-k-c", false)); - assertEquals("very-long-kebab-case", match(set, "vLKC", false)); - assertEquals("camelCase", match(set, "camelCase", false)); - assertEquals("camelCase", match(set, "cC", false)); - assertEquals("camelCase", match(set, "c-c", false)); - assertEquals("camelCase", match(set, "camC", false)); - assertEquals("camelCase", match(set, "camel", false)); - assertEquals("camelCase", match(set, "ca", false)); - assertEquals("super-long-option", match(set, "s-l-o", false)); - assertEquals("super-long-option", match(set, "s-long-opt", false)); - assertEquals("super-long-option", match(set, "super", false)); - assertEquals("super-long-option", match(set, "sup", false)); - assertEquals("super-long-option", match(set, "sup-long", false)); - assertNotEquals("super-long-option", match(set, "SLO", false)); - assertEquals ("super-long-option", match(set, "sLO", false)); - assertEquals("super-long-option", match(set, "s-opt", false)); - assertEquals("veryLongCamelCase", match(set, "veryLongCamelCase", false)); - assertEquals("veryLongCamelCase", match(set, "vLCC", false)); - assertEquals("veryLongCamelCase", match(set, "v-l-c-c", false)); - assertEquals("extremely-long-option-with-many-components", match(set, "extr-opt-comp", false)); - assertEquals("extremely-long-option-with-many-components", match(set, "extr-long-opt-comp", false)); - assertEquals("extremely-long-option-with-many-components", match(set, "extr-comp", false)); - assertNotEquals("extremely-long-option-with-many-components", match(set, "extr-opt-long-comp", false)); - assertNotEquals("extremely-long-option-with-many-components", match(set, "long", false)); + CommandLine cmd = new CommandLine(Model.CommandSpec.create()); + + assertEquals("kebab-case", match(set, "kebab-case", false, cmd)); + assertEquals("kebab-case-extra", match(set, "kebab-case-extra", false, cmd)); + assertEquals("very-long-kebab-case", match(set, "very-long-kebab-case", false, cmd)); + assertEquals("very-long-kebab-case", match(set, "v-l-k-c", false, cmd)); + assertEquals("very-long-kebab-case", match(set, "vLKC", false, cmd)); + assertEquals("camelCase", match(set, "camelCase", false, cmd)); + assertEquals("camelCase", match(set, "cC", false, cmd)); + assertEquals("camelCase", match(set, "c-c", false, cmd)); + assertEquals("camelCase", match(set, "camC", false, cmd)); + assertEquals("camelCase", match(set, "camel", false, cmd)); + assertEquals("camelCase", match(set, "ca", false, cmd)); + assertEquals("super-long-option", match(set, "s-l-o", false, cmd)); + assertEquals("super-long-option", match(set, "s-long-opt", false, cmd)); + assertEquals("super-long-option", match(set, "super", false, cmd)); + assertEquals("super-long-option", match(set, "sup", false, cmd)); + assertEquals("super-long-option", match(set, "sup-long", false, cmd)); + assertNotEquals("super-long-option", match(set, "SLO", false, cmd)); + assertEquals ("super-long-option", match(set, "sLO", false, cmd)); + assertEquals("super-long-option", match(set, "s-opt", false, cmd)); + assertEquals("veryLongCamelCase", match(set, "veryLongCamelCase", false, cmd)); + assertEquals("veryLongCamelCase", match(set, "vLCC", false, cmd)); + assertEquals("veryLongCamelCase", match(set, "v-l-c-c", false, cmd)); + assertEquals("extremely-long-option-with-many-components", match(set, "extr-opt-comp", false, cmd)); + assertEquals("extremely-long-option-with-many-components", match(set, "extr-long-opt-comp", false, cmd)); + assertEquals("extremely-long-option-with-many-components", match(set, "extr-comp", false, cmd)); + assertNotEquals("extremely-long-option-with-many-components", match(set, "extr-opt-long-comp", false, cmd)); + assertNotEquals("extremely-long-option-with-many-components", match(set, "long", false, cmd)); try { - match(set, "vLC", false); + match(set, "vLC", false, cmd); fail("Expected exception"); - } catch (IllegalArgumentException ex) { - assertEquals("'vLC' is not unique: it matches 'very-long-kebab-case', 'veryLongCamelCase'", ex.getMessage()); + } catch (ParameterException ex) { + assertEquals("Error: 'vLC' is not unique: it matches 'very-long-kebab-case', 'veryLongCamelCase'", ex.getMessage()); } try { - match(set, "k-c", false); + match(set, "k-c", false, cmd); fail("Expected exception"); - } catch (IllegalArgumentException ex) { - assertEquals("'k-c' is not unique: it matches 'kebab-case-extra', 'kebab-case-extra-extra', 'kebab-case'", ex.getMessage()); + } catch (ParameterException ex) { + assertEquals("Error: 'k-c' is not unique: it matches 'kebab-case-extra', 'kebab-case-extra-extra', 'kebab-case'", ex.getMessage()); } try { - match(set, "kC", false); + match(set, "kC", false, cmd); fail("Expected exception"); - } catch (IllegalArgumentException ex) { - assertEquals("'kC' is not unique: it matches 'kebab-case-extra', 'kebab-case-extra-extra', 'kebab-case'", ex.getMessage()); + } catch (ParameterException ex) { + assertEquals("Error: 'kC' is not unique: it matches 'kebab-case-extra', 'kebab-case-extra-extra', 'kebab-case'", ex.getMessage()); } try { - match(set, "keb-ca", false); + match(set, "keb-ca", false, cmd); fail("Expected exception"); - } catch (IllegalArgumentException ex) { - assertEquals("'keb-ca' is not unique: it matches 'kebab-case-extra', 'kebab-case-extra-extra', 'kebab-case'", ex.getMessage()); + } catch (ParameterException ex) { + assertEquals("Error: 'keb-ca' is not unique: it matches 'kebab-case-extra', 'kebab-case-extra-extra', 'kebab-case'", ex.getMessage()); } } @@ -107,26 +115,28 @@ public void testUserManualExamples() { original.add("--super-long-option"); original.add("some-long-command"); - assertNotEquals("--veryLongCamelCase", match(original, "--VLCC", false)); - assertEquals("--veryLongCamelCase", match(original, "--very", false)); - assertEquals("--veryLongCamelCase", match(original, "--vLCC", false)); - assertEquals("--veryLongCamelCase", match(original, "--vCase", false)); - - assertEquals("--super-long-option", match(original, "--sup", false)); - assertNotEquals("--super-long-option", match(original, "--Sup", false)); - assertEquals("--super-long-option", match(original, "--sLO", false)); - assertEquals("--super-long-option", match(original, "--s-l-o", false)); - assertEquals("--super-long-option", match(original, "--s-lon", false)); - assertEquals("--super-long-option", match(original, "--s-opt", false)); - assertEquals("--super-long-option", match(original, "--sOpt", false)); - - assertNotEquals("some-long-command", match(original, "So", false)); - assertEquals("some-long-command", match(original, "so", false)); - assertEquals("some-long-command", match(original, "sLC", false)); - assertEquals("some-long-command", match(original, "s-l-c", false)); - assertEquals("some-long-command", match(original, "soLoCo", false)); - assertNotEquals("some-long-command", match(original, "SoLoCo", false)); - assertEquals("some-long-command", match(original, "someCom", false)); + CommandLine cmd = new CommandLine(Model.CommandSpec.create()); + + assertNotEquals("--veryLongCamelCase", match(original, "--VLCC", false, cmd)); + assertEquals("--veryLongCamelCase", match(original, "--very", false, cmd)); + assertEquals("--veryLongCamelCase", match(original, "--vLCC", false, cmd)); + assertEquals("--veryLongCamelCase", match(original, "--vCase", false, cmd)); + + assertEquals("--super-long-option", match(original, "--sup", false, cmd)); + assertNotEquals("--super-long-option", match(original, "--Sup", false, cmd)); + assertEquals("--super-long-option", match(original, "--sLO", false, cmd)); + assertEquals("--super-long-option", match(original, "--s-l-o", false, cmd)); + assertEquals("--super-long-option", match(original, "--s-lon", false, cmd)); + assertEquals("--super-long-option", match(original, "--s-opt", false, cmd)); + assertEquals("--super-long-option", match(original, "--sOpt", false, cmd)); + + assertNotEquals("some-long-command", match(original, "So", false, cmd)); + assertEquals("some-long-command", match(original, "so", false, cmd)); + assertEquals("some-long-command", match(original, "sLC", false, cmd)); + assertEquals("some-long-command", match(original, "s-l-c", false, cmd)); + assertEquals("some-long-command", match(original, "soLoCo", false, cmd)); + assertNotEquals("some-long-command", match(original, "SoLoCo", false, cmd)); + assertEquals("some-long-command", match(original, "someCom", false, cmd)); } @Test @@ -538,4 +548,19 @@ class App { assertEquals("Unknown option: '--AB'", ex.getMessage()); } } + + @Test + public void testAbbreviatedOptionWithAttachedValue() { + @Command + class Bean { + @Option(names = "--xxx-yyy") + int x; + } + Bean bean = new Bean(); + CommandLine cmd = new CommandLine(bean); + cmd.setAbbreviatedOptionsAllowed(true); + cmd.parseArgs("--x-y=123"); + assertEquals(123, bean.x); + } + } diff --git a/src/test/java/picocli/InheritedOptionTest.java b/src/test/java/picocli/InheritedOptionTest.java index 32242420a..d0d78dc2d 100644 --- a/src/test/java/picocli/InheritedOptionTest.java +++ b/src/test/java/picocli/InheritedOptionTest.java @@ -492,4 +492,16 @@ public void testIssue1159() { assertEquals(345, bean.x); } + @Test + public void testIssue1159WithEquals() { + Issue1159 bean = new Issue1159(); + CommandLine cmd = new CommandLine(bean); + cmd.setAbbreviatedOptionsAllowed(true); + cmd.parseArgs("--x-y=123"); + assertEquals(123, bean.x); + + cmd.parseArgs("sub", "--x-y=345"); + assertEquals(345, bean.x); + } + }