Skip to content

Commit

Permalink
[remkop#1537] AbbreviationMatcher now treats aliases of the same obje…
Browse files Browse the repository at this point in the history 
…ct as one match

Fixes remkop#1537.
  • Loading branch information
@NewbieOrange
NewbieOrange committed Jan 3, 2022
1 parent ce28ef9 commit 20275d0
Show file tree
Hide file tree
Showing 3 changed files with 183 additions and 152 deletions.
136 changes: 99 additions & 37 deletions src/main/java/picocli/CommandLine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6478,7 +6478,7 @@ private void updatedSubcommandsToInheritFrom(CommandSpec root) {
public CommandLine removeSubcommand(String name) {
String actualName = name;
if (parser().abbreviatedSubcommandsAllowed()) {
actualName = AbbreviationMatcher.match(commands.keySet(), name, subcommandsCaseInsensitive(), commandLine);
actualName = AbbreviationMatcher.match(commands, name, subcommandsCaseInsensitive(), commandLine).getFullName();
}

Set<String> removedNames = new TreeSet<String>();
Expand Down Expand Up @@ -13316,43 +13316,52 @@ private void processArguments(List<CommandLine> parsedCommands,
}

// if we find another command, we are done with the current command
if (commandSpec.parser().abbreviatedSubcommandsAllowed()) {
arg = AbbreviationMatcher.match(commandSpec.subcommands().keySet(), arg, commandSpec.subcommandsCaseInsensitive(), CommandLine.this);
CommandLine subcommand = commandSpec.subcommands().get(arg);
if (subcommand == null && commandSpec.parser().abbreviatedSubcommandsAllowed()) {
subcommand = AbbreviationMatcher.match(commandSpec.subcommands(), arg, commandSpec.subcommandsCaseInsensitive(), CommandLine.this).getValue();
}
if (commandSpec.subcommands().containsKey(arg)) {
CommandLine subcommand = commandSpec.subcommands().get(arg);
if (subcommand != null) {
processSubcommand(subcommand, parseResultBuilder, parsedCommands, args, required, initialized, originalArgs, nowProcessing, separator, arg);
return; // remainder done by the command
}
if (commandSpec.parent() != null && commandSpec.parent().subcommandsRepeatable() && commandSpec.parent().subcommands().containsKey(arg)) {
tracer.debug("'%s' is a repeatable subcommand of %s%n", arg, commandSpec.parent().qualifiedName());// #454 repeatable subcommands
CommandLine subcommand = commandSpec.parent().subcommands().get(arg);
Set<ArgSpec> inheritedInitialized = initialized;
if (subcommand.interpreter.parseResultBuilder != null) {
tracer.debug("Subcommand '%s' has been matched before. Making a copy...%n", subcommand.getCommandName());
subcommand = subcommand.copy();
subcommand.getCommandSpec().parent(commandSpec.parent()); // hook it up with its parent
inheritedInitialized = new LinkedHashSet<ArgSpec>(inheritedInitialized);
CommandSpec parent = commandSpec.parent();
if (parent != null && parent.subcommandsRepeatable()) {
subcommand = parent.subcommands().get(arg);
if (subcommand == null && parent.parser().abbreviatedSubcommandsAllowed()) {

This comment has been minimized.

Sign in to view

Copy link
@NewbieOrange

NewbieOrange Jan 3, 2022

Author Owner

Previously we seemed to forget checking abbreviation for parent CommandSpec?
subcommand = AbbreviationMatcher.match(parent.subcommands(), arg, parent.subcommandsCaseInsensitive(), CommandLine.this).getValue();
}
if (subcommand != null) {
tracer.debug("'%s' is a repeatable subcommand of %s%n", arg,
commandSpec.parent().qualifiedName()); // #454 repeatable subcommands
Set<ArgSpec> inheritedInitialized = initialized;
if (subcommand.interpreter.parseResultBuilder != null) {
tracer.debug("Subcommand '%s' has been matched before. Making a copy...%n",
subcommand.getCommandName());
subcommand = subcommand.copy();
subcommand.getCommandSpec().parent(commandSpec.parent()); // hook it up with its parent
inheritedInitialized = new LinkedHashSet<ArgSpec>(inheritedInitialized);
}
processSubcommand(subcommand, getParent().interpreter.parseResultBuilder, parsedCommands, args,
required, inheritedInitialized, originalArgs, nowProcessing, separator, arg);
continue;
}
processSubcommand(subcommand, getParent().interpreter.parseResultBuilder, parsedCommands, args, required, inheritedInitialized, originalArgs, nowProcessing, separator, arg);
continue;
}

// First try to interpret the argument as a single option (as opposed to a compact group of options).
// 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<String> aggregatedOptionNames = new LinkedHashSet<String>();
LinkedHashMap<String, OptionSpec> aggregatedOptionNames = new LinkedHashMap<String, OptionSpec>();
if (commandSpec.parser().abbreviatedOptionsAllowed()) {
aggregatedOptionNames.addAll(commandSpec.optionsMap().keySet());
aggregatedOptionNames.addAll(commandSpec.negatedOptionsMap().keySet());
arg = AbbreviationMatcher.match(aggregatedOptionNames, arg, commandSpec.optionsCaseInsensitive(), CommandLine.this);
aggregatedOptionNames.putAll(commandSpec.optionsMap());
aggregatedOptionNames.putAll(commandSpec.negatedOptionsMap());
arg = AbbreviationMatcher.match(aggregatedOptionNames, arg, commandSpec.optionsCaseInsensitive(), CommandLine.this).getFullName();
}
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
key = AbbreviationMatcher.match(aggregatedOptionNames, key, commandSpec.optionsCaseInsensitive(), CommandLine.this).getFullName(); //#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);
Expand Down Expand Up @@ -14819,12 +14828,11 @@ public void run() {
Help.ColorScheme colors = colorScheme != null ? colorScheme : Help.defaultColorScheme(ansi);
if (commands != null) {
Map<String, CommandLine> parentSubcommands = parent.getCommandSpec().subcommands();
String fullName = commands;
if (parent.isAbbreviatedSubcommandsAllowed()) {
fullName = AbbreviationMatcher.match(parentSubcommands.keySet(), fullName,
parent.isSubcommandsCaseInsensitive(), self);
CommandLine subcommand = parentSubcommands.get(commands);
if (subcommand == null && parent.isAbbreviatedSubcommandsAllowed()) {
subcommand = AbbreviationMatcher.match(parentSubcommands, commands,
parent.isSubcommandsCaseInsensitive(), self).getValue();
}
CommandLine subcommand = parentSubcommands.get(fullName);
if (subcommand != null) {
if (outWriter != null) {
subcommand.usage(outWriter, colors);
Expand Down Expand Up @@ -18378,6 +18386,42 @@ private String getValue(String key, CommandSpec spec) {
}

static class AbbreviationMatcher {
static class MatchResult<V> {
private final String fullName;
private final V value;

MatchResult(String fullName, V value) {
this.fullName = fullName;
this.value = value;
}

String getFullName() {
return fullName;
}

boolean hasValue() {
return value != null;
}

V getValue() {
return value;
}

@Override
public boolean equals(Object o) {
if (!(o instanceof MatchResult)) {
return false;
}
MatchResult<?> p = (MatchResult<?>) o;
return fullName.equals(p.fullName) && Objects.equals(value, p.value);
}

@Override
public int hashCode() {
return fullName.hashCode() ^ (value == null ? 0 : value.hashCode());
}
}

public static List<String> splitIntoChunks(String command, boolean caseInsensitive) {
List<String> result = new ArrayList<String>();
int start = 0, codepoint;
Expand Down Expand Up @@ -18426,25 +18470,29 @@ private static String makeCanonical(String str) {
return str;
}

/** Returns the non-abbreviated name if found, otherwise returns the specified original abbreviation value. */
public static String match(Set<String> set, String abbreviation, boolean caseInsensitive, CommandLine source) {
if (set.contains(abbreviation) || set.isEmpty()) { // return exact match
return abbreviation;
/** Returns the non-abbreviated name if found, otherwise returns the specified original abbreviation name. */
public static <T> MatchResult<T> match(Map<String, T> map, String abbreviation, boolean caseInsensitive, CommandLine source) {
MatchResult<T> result = new MatchResult<T>(abbreviation, map.get(abbreviation));
if (result.hasValue() || map.isEmpty()) {
return result;
}
List<String> abbreviatedKeyChunks = splitIntoChunks(abbreviation, caseInsensitive);
List<String> candidates = new ArrayList<String>();
for (String key : set) {
List<String> keyChunks = splitIntoChunks(key, caseInsensitive);
Map<String, T> candidates = new LinkedHashMap<String, T>();
for (Map.Entry<String, T> entry : map.entrySet()) {
List<String> keyChunks = splitIntoChunks(entry.getKey(), caseInsensitive);
if (matchKeyChunks(abbreviatedKeyChunks, keyChunks, caseInsensitive)) {
candidates.add(key);
if (!result.hasValue()) {
result = new MatchResult<T>(entry.getKey(), entry.getValue());
}
candidates.put(entry.getKey(), entry.getValue());
}
}
if (candidates.size() > 1) {
String str = candidates.toString();
if (!isAllCandidatesSame(candidates.values())) {
String str = candidates.keySet().toString();
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
return result; // return the original with null as value if no match found
}

private static boolean matchKeyChunks(List<String> abbreviatedKeyChunks, List<String> keyChunks, boolean caseInsensitive) {
Expand Down Expand Up @@ -18492,5 +18540,19 @@ private static boolean isNonAlphabetic(String str) {
}
return true;
}

private static <T> boolean isAllCandidatesSame(Collection<T> candidates) {
if (candidates.isEmpty()) {
return true;
}
Iterator<T> iterator = candidates.iterator();
T first = iterator.next();
while (iterator.hasNext()) {
if (iterator.next() != first) { // check reference equality as aliases point to the same object
return false;
}
}
return true;
}
}
}

0 comments on commit 20275d0

Please sign in to comment.