diff --git a/src/main/java/org/springframework/shell/core/Converter.java b/src/main/java/org/springframework/shell/core/Converter.java index b8b5e004..16a9718c 100644 --- a/src/main/java/org/springframework/shell/core/Converter.java +++ b/src/main/java/org/springframework/shell/core/Converter.java @@ -20,38 +20,39 @@ import java.util.List; import org.springframework.shell.core.annotation.CliCommand; import org.springframework.shell.core.annotation.CliOption; - /** * Converts between Strings (as displayed by and entered via the shell) and Java objects - * + * * @author Ben Alex * @param the type being converted to/from */ public interface Converter { + /** + * The prefix for the option context property that indicates how many successive tab completion requests have + * occurred. + */ + public static final String TAB_COMPLETION_COUNT_PREFIX = "tab-completion-count-"; + /** * Indicates whether this converter supports the given type in the given option context - * + * * @param type the type being checked - * @param optionContext a non-null string that customises the - * behaviour of this converter for a given {@link CliOption} of a given - * {@link CliCommand}; the contents will have special meaning to this - * converter (e.g. be a comma-separated list of keywords known to this - * converter) + * @param optionContext a non-null string that customises the behaviour of this converter for a given + * {@link CliOption} of a given {@link CliCommand}; the contents will have special meaning to this converter (e.g. + * be a comma-separated list of keywords known to this converter) * @return see above */ boolean supports(Class type, String optionContext); /** * Converts from the given String value to type T - * + * * @param value the value to convert * @param targetType the type being converted to; can't be null - * @param optionContext a non-null string that customises the - * behaviour of this converter for a given {@link CliOption} of a given - * {@link CliCommand}; the contents will have special meaning to this - * converter (e.g. be a comma-separated list of keywords known to this - * converter) + * @param optionContext a non-null string that customises the behaviour of this converter for a given + * {@link CliOption} of a given {@link CliCommand}; the contents will have special meaning to this converter (e.g. + * be a comma-separated list of keywords known to this converter) * @return see above * @throws RuntimeException if the given value could not be converted */ @@ -59,19 +60,17 @@ public interface Converter { /** * Populates the given list with the possible completions - * + * * @param completions the list to populate; can't be null * @param targetType the type of parameter for which a string is being entered * @param existingData what the user has typed so far - * @param optionContext a non-null string that customises the - * behaviour of this converter for a given {@link CliOption} of a given - * {@link CliCommand}; the contents will have special meaning to this - * converter (e.g. be a comma-separated list of keywords known to this - * converter) + * @param optionContext a non-null string that customises the behaviour of this converter for a given + * {@link CliOption} of a given {@link CliCommand}; the contents will have special meaning to this converter (e.g. + * be a comma-separated list of keywords known to this converter) * @param target - * @return true if all the added completions are complete - * values, or false if the user can press TAB to add further - * information to some or all of them + * @return true if all the added completions are complete values, or false if the user can + * press TAB to add further information to some or all of them */ - boolean getAllPossibleValues(List completions, Class targetType, String existingData, String optionContext, MethodTarget target); + boolean getAllPossibleValues(List completions, Class targetType, String existingData, + String optionContext, MethodTarget target); } diff --git a/src/main/java/org/springframework/shell/core/SimpleParser.java b/src/main/java/org/springframework/shell/core/SimpleParser.java index 9be2c648..7f963905 100644 --- a/src/main/java/org/springframework/shell/core/SimpleParser.java +++ b/src/main/java/org/springframework/shell/core/SimpleParser.java @@ -65,6 +65,16 @@ public class SimpleParser implements Parser { private final Map availabilityIndicators = new HashMap(); + /** + * The last buffer when completion was requested. + */ + private String previousCompletionBuffer; + + /** + * The number of times completion has been requested with the same buffer. + */ + private int successiveCompletionRequests = 1; + private MethodTarget getAvailabilityIndicator(final String command) { return availabilityIndicators.get(command); } @@ -109,6 +119,8 @@ public class SimpleParser implements Parser { Assert.notNull(rawInput, "Raw input required"); final String input = normalise(rawInput); + resetCompletionInvocations(); + // Locate the applicable targets which match this buffer final Collection matchingTargets = locateTargets(input, true, true); if (matchingTargets.isEmpty()) { @@ -315,6 +327,15 @@ public class SimpleParser implements Parser { } } + /** + * We're being asked to execute a command, so the next completion invocation will definitely refer to a different + * buffer. + */ + private void resetCompletionInvocations() { + previousCompletionBuffer = null; + successiveCompletionRequests = 1; + } + private void reportTokenizingException(String commandKey, TokenizingException te) { StringBuilder caret = new StringBuilder(); for (int i = 0; i < te.getOffendingOffset() + commandKey.length() + 1; i++) { @@ -565,15 +586,11 @@ public class SimpleParser implements Parser { cursor--; } - // Replace all multiple spaces with a single space - while (buffer.contains(" ")) { - buffer = buffer.replaceFirst(" ", " "); - cursor--; - } - // Begin by only including the portion of the buffer represented to the present cursor position String translated = buffer.substring(0, cursor); + String successiveInvocationContext = trackSuccessiveCompletionRequests(translated); + // Start by locating a method that matches final Collection targets = locateTargets(translated, false, true); SortedSet results = new TreeSet(COMPARATOR); @@ -834,10 +851,11 @@ public class SimpleParser implements Parser { } // Let's use a Converter if one is available for (Converter candidate : converters) { - if (candidate.supports(parameterType, option.optionContext())) { + String optionContext = successiveInvocationContext + " " + option.optionContext(); + if (candidate.supports(parameterType, optionContext)) { // Found a usable converter boolean allComplete = candidate.getAllPossibleValues(allValues, parameterType, - lastOptionValue, option.optionContext(), methodTarget); + lastOptionValue, optionContext, methodTarget); if (!allComplete) { suffix = ""; } @@ -892,6 +910,22 @@ public class SimpleParser implements Parser { } } + /** + * Track the number of times completion has been requested for the same buffer, resetting everytime the buffer + * changes. + * @return the portion of "option context" that indicates the number of invocation + */ + private String trackSuccessiveCompletionRequests(String translated) { + if (translated.equals(previousCompletionBuffer)) { + successiveCompletionRequests++; + } + else { + previousCompletionBuffer = translated; + successiveCompletionRequests = 1; + } + return Converter.TAB_COMPLETION_COUNT_PREFIX + successiveCompletionRequests; + } + private void displayHelp(String lastOptionKey, CliOption option) { StringBuilder help = new StringBuilder(); help.append(OsUtils.LINE_SEPARATOR); diff --git a/src/test/java/org/springframework/shell/core/SimpleParserTests.java b/src/test/java/org/springframework/shell/core/SimpleParserTests.java index 42e754a0..46ce700d 100644 --- a/src/test/java/org/springframework/shell/core/SimpleParserTests.java +++ b/src/test/java/org/springframework/shell/core/SimpleParserTests.java @@ -30,10 +30,12 @@ import static org.junit.Assert.assertThat; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.regex.Pattern; import org.hamcrest.Description; import org.hamcrest.DiagnosingMatcher; import org.hamcrest.Matcher; +import org.junit.Assert; import org.junit.Test; import org.springframework.shell.core.annotation.CliCommand; import org.springframework.shell.core.annotation.CliOption; @@ -369,6 +371,46 @@ public class SimpleParserTests { assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 \"def\" "))))); } + @Test + public void testSuccessiveInvocationsOfCompletion() { + + parser.add(new MyCommands()); + parser.add(new ExpertCompletions()); + + buffer = "bar --option1 "; + offset = parser.completeAdvanced(buffer, buffer.length(), candidates); + + assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 one "))))); + assertThat(candidates, not(hasItem(completionThat(is(equalTo("bar --option1 two ")))))); + assertThat(candidates, not(hasItem(completionThat(is(equalTo("bar --option1 three ")))))); + + candidates.clear(); + // Simulate twice + offset = parser.completeAdvanced(buffer, buffer.length(), candidates); + + assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 one "))))); + assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 two "))))); + assertThat(candidates, not(hasItem(completionThat(is(equalTo("bar --option1 three ")))))); + + candidates.clear(); + // Simulate three times + offset = parser.completeAdvanced(buffer, buffer.length(), candidates); + + assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 one "))))); + assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 two "))))); + assertThat(candidates, hasItem(completionThat(is(equalTo("bar --option1 three "))))); + + // And now for something completely different + buffer = "testMandatory --option2 "; + candidates.clear(); + offset = parser.completeAdvanced(buffer, buffer.length(), candidates); + + assertThat(candidates, hasItem(completionThat(is(equalTo("testMandatory --option2 one "))))); + assertThat(candidates, not(hasItem(completionThat(is(equalTo("testMandatory --option2 two ")))))); + assertThat(candidates, not(hasItem(completionThat(is(equalTo("testMandatory --option2 three ")))))); + + } + /** * @see https://jira.spring.io/browse/SHL-113 */ @@ -517,4 +559,34 @@ public class SimpleParserTests { } + public static class ExpertCompletions implements Converter { + + private static final Pattern NUMBER_OF_COMPLETIONS_CAPTURE = Pattern.compile(".*completion-count-(\\d+).*"); + + private static final String[] results = new String[] { "one", "two", "three" }; + + @Override + public boolean supports(Class type, String optionContext) { + return true; + } + + @Override + public String convertFromText(String value, Class targetType, String optionContext) { + return value; + } + + @Override + public boolean getAllPossibleValues(List completions, Class targetType, String existingData, + String optionContext, MethodTarget target) { + java.util.regex.Matcher m = NUMBER_OF_COMPLETIONS_CAPTURE.matcher(optionContext); + Assert.assertTrue(m.matches()); + int invocations = Integer.parseInt(m.group(1)); + for (int i = 0; i < invocations; i++) { + completions.add(new Completion(results[i])); + } + return true; + } + + } + }