diff --git a/src/main/java/org/springframework/shell/core/SimpleParser.java b/src/main/java/org/springframework/shell/core/SimpleParser.java index 23a74e4f..0c06e257 100644 --- a/src/main/java/org/springframework/shell/core/SimpleParser.java +++ b/src/main/java/org/springframework/shell/core/SimpleParser.java @@ -658,7 +658,7 @@ public class SimpleParser implements Parser { // Handle if they are trying to find out the available option keys; always present option keys in order // of their declaration on the method signature, thus we can stop when mandatory options are filled in - if (methodTarget.getRemainingBuffer().endsWith("--") && !tokenizer.lastValueIsStillBeingTyped()) { + if (methodTarget.getRemainingBuffer().endsWith("--") && !tokenizer.openingQuotesHaveNotBeenClosed()) { boolean showAllRemaining = true; for (CliOption include : unspecified) { if (include.mandatory()) { @@ -685,7 +685,7 @@ public class SimpleParser implements Parser { // option key/value pair) if (lastOptionKey == null || (!"".equals(lastOptionKey) && !"".equals(lastOptionValue) && translated.endsWith(" ") && !tokenizer - .lastValueIsStillBeingTyped())) { + .openingQuotesHaveNotBeenClosed())) { // We have either NEVER specified an option key/value pair // OR we have specified a full option key/value pair @@ -740,8 +740,8 @@ public class SimpleParser implements Parser { } } - // Only abort at this point if we have some suggestions; otherwise we might want to try to complete the - // "" option + // Only abort at this point if we have some suggestions; + // otherwise we might want to try to complete the "" option if (results.size() > 0) { candidates.addAll(results); return 0; @@ -749,13 +749,12 @@ public class SimpleParser implements Parser { } // Handle completing the option key they're presently typing - if ("".equals(lastOptionValue)) { + if (lastOptionKey != null && "".equals(lastOptionValue)) { // Given we haven't got an option value of any form, we must // still be typing an option key. - for (CliOption option : cliOptions) { + for (CliOption option : unspecified) { for (String value : option.key()) { - if (value != null && lastOptionKey != null - && value.regionMatches(true, 0, lastOptionKey, 0, lastOptionKey.length())) { + if (value != null && value.regionMatches(true, 0, lastOptionKey, 0, lastOptionKey.length())) { String completionValue = translated.substring(0, (translated.length() - lastOptionKey.length())) + value + " "; @@ -795,85 +794,26 @@ public class SimpleParser implements Parser { } if (allValues.isEmpty()) { - // Doesn't appear to be a custom Converter, so let's go and provide defaults for simple - // types - - // Provide some simple options for common types - if (Boolean.class.isAssignableFrom(parameterType) - || Boolean.TYPE.isAssignableFrom(parameterType)) { - allValues.add(new Completion("true")); - allValues.add(new Completion("false")); - } - - if (Number.class.isAssignableFrom(parameterType)) { - allValues.add(new Completion("0")); - allValues.add(new Completion("1")); - allValues.add(new Completion("2")); - allValues.add(new Completion("3")); - allValues.add(new Completion("4")); - allValues.add(new Completion("5")); - allValues.add(new Completion("6")); - allValues.add(new Completion("7")); - allValues.add(new Completion("8")); - allValues.add(new Completion("9")); - } + // Doesn't appear to be a custom Converter, so let's go and provide defaults + // for simple types + completeForSimpleTypes(parameterType, allValues); } // Only include in the candidates those results which are compatible with the present buffer for (Completion currentValue : allValues) { - // We only provide a suggestion if the lastOptionValue == "" - if (!StringUtils.hasText(lastOptionValue)) { - // We should add the result, as they haven't typed anything yet + // Only add the result **if** what they've typed is compatible *AND* they haven't + // already typed it in full + if (currentValue.getValue().toLowerCase().startsWith(lastOptionValue.toLowerCase()) + && lastOptionValue.length() < currentValue.getValue().length() + && (!tokenizer.lastValueIsComplete())) { results.add(new Completion(currentValue.getValue() + suffix, currentValue .getFormattedValue(), currentValue.getHeading(), currentValue.getOrder())); } - else { - // Only add the result **if** what they've typed is compatible *AND* they haven't - // already typed it in full - if (currentValue.getValue().toLowerCase().startsWith(lastOptionValue.toLowerCase()) - && !lastOptionValue.equalsIgnoreCase(currentValue.getValue()) - && lastOptionValue.length() < currentValue.getValue().length() - && (tokenizer.getLastValueDelimiter() == ' ' || tokenizer - .lastValueIsStillBeingTyped())) { - results.add(new Completion(currentValue.getValue() + suffix, currentValue - .getFormattedValue(), currentValue.getHeading(), currentValue - .getOrder())); - } - } } // ROO-389: give inline options given there's multiple choices available and we want to help // the user - StringBuilder help = new StringBuilder(); - help.append(OsUtils.LINE_SEPARATOR); - help.append(option.mandatory() ? "required --" : "optional --"); - if ("".equals(option.help())) { - help.append(lastOptionKey).append(": ").append("No help available"); - } - else { - help.append(lastOptionKey).append(": ").append(option.help()); - } - if (option.specifiedDefaultValue().equals(option.unspecifiedDefaultValue())) { - if (option.specifiedDefaultValue().equals("__NULL__")) { - help.append("; no default value"); - } - else { - help.append("; default: '").append(option.specifiedDefaultValue()).append("'"); - } - } - else { - if (!"".equals(option.specifiedDefaultValue()) - && !"__NULL__".equals(option.specifiedDefaultValue())) { - help.append("; default if option present: '") - .append(option.specifiedDefaultValue()).append("'"); - } - if (!"".equals(option.unspecifiedDefaultValue()) - && !"__NULL__".equals(option.unspecifiedDefaultValue())) { - help.append("; default if option not present: '") - .append(option.unspecifiedDefaultValue()).append("'"); - } - } - LOGGER.info(help.toString()); + displayHelp(lastOptionKey, option); if (results.size() == 1) { String suggestion = results.iterator().next().getValue().trim(); @@ -899,6 +839,56 @@ public class SimpleParser implements Parser { } } + private void displayHelp(String lastOptionKey, CliOption option) { + StringBuilder help = new StringBuilder(); + help.append(OsUtils.LINE_SEPARATOR); + help.append(option.mandatory() ? "required --" : "optional --"); + if ("".equals(option.help())) { + help.append(lastOptionKey).append(": ").append("No help available"); + } + else { + help.append(lastOptionKey).append(": ").append(option.help()); + } + if (option.specifiedDefaultValue().equals(option.unspecifiedDefaultValue())) { + if (option.specifiedDefaultValue().equals("__NULL__")) { + help.append("; no default value"); + } + else { + help.append("; default: '").append(option.specifiedDefaultValue()).append("'"); + } + } + else { + if (!"".equals(option.specifiedDefaultValue()) && !"__NULL__".equals(option.specifiedDefaultValue())) { + help.append("; default if option present: '").append(option.specifiedDefaultValue()).append("'"); + } + if (!"".equals(option.unspecifiedDefaultValue()) && !"__NULL__".equals(option.unspecifiedDefaultValue())) { + help.append("; default if option not present: '").append(option.unspecifiedDefaultValue()).append("'"); + } + } + LOGGER.info(help.toString()); + } + + private void completeForSimpleTypes(Class parameterType, List allValues) { + // Provide some simple options for common types + if (Boolean.class.isAssignableFrom(parameterType) || Boolean.TYPE.isAssignableFrom(parameterType)) { + allValues.add(new Completion("true")); + allValues.add(new Completion("false")); + } + + if (Number.class.isAssignableFrom(parameterType)) { + allValues.add(new Completion("0")); + allValues.add(new Completion("1")); + allValues.add(new Completion("2")); + allValues.add(new Completion("3")); + allValues.add(new Completion("4")); + allValues.add(new Completion("5")); + allValues.add(new Completion("6")); + allValues.add(new Completion("7")); + allValues.add(new Completion("8")); + allValues.add(new Completion("9")); + } + } + /** * populate completion for mandatory options * diff --git a/src/main/java/org/springframework/shell/core/Tokenizer.java b/src/main/java/org/springframework/shell/core/Tokenizer.java index e9d66919..b8f7592a 100644 --- a/src/main/java/org/springframework/shell/core/Tokenizer.java +++ b/src/main/java/org/springframework/shell/core/Tokenizer.java @@ -54,7 +54,7 @@ public class Tokenizer { /** * Used to indicate that the last value was indeed half enclosed in quotes. Useful so that parser can re-add it. */ - private boolean lastValueIsStillBeingTyped; + private boolean openingQuotesHaveNotBeenClosed; private char lastValueDelimiter; @@ -137,7 +137,7 @@ public class Tokenizer { (buffer[pos - 1] != '"' || // quotes are not properly closed sb.length() == 0)) { // BUT it's ok if consumed nothing (pos-1 is *opening* quote then) if (allowUnbalancedLastQuotedValue) { - lastValueIsStillBeingTyped = true; + openingQuotesHaveNotBeenClosed = true; return sb.toString(); } else { @@ -166,8 +166,17 @@ public class Tokenizer { /** * Return whether the last value was meant to be enclosed in quotes, but the closing quote has not been typed yet. */ - public boolean lastValueIsStillBeingTyped() { - return lastValueIsStillBeingTyped; + public boolean openingQuotesHaveNotBeenClosed() { + return openingQuotesHaveNotBeenClosed; + } + + /** + * Return true if we know for sure that the last value has been typed in full. + */ + public boolean lastValueIsComplete() { + // If using quotes as delim and they're not closed, we know for sure. + // Moreover if we're using space as the delimiter, we can't know. + return !openingQuotesHaveNotBeenClosed && lastValueDelimiter != ' '; } /** diff --git a/src/test/java/org/springframework/shell/core/SimpleParserTests.java b/src/test/java/org/springframework/shell/core/SimpleParserTests.java index c4e8faf4..cd4ed881 100644 --- a/src/test/java/org/springframework/shell/core/SimpleParserTests.java +++ b/src/test/java/org/springframework/shell/core/SimpleParserTests.java @@ -16,7 +16,9 @@ package org.springframework.shell.core; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; @@ -138,6 +140,17 @@ public class SimpleParserTests { assertThat(candidates, is(empty())); } + @Test + public void testArgumentValueCompletionAlreadyGiven() { + parser.add(new MyCommands()); + parser.add(new StringCompletions(Arrays.asList("abc", "def", "ghi"))); + + buffer = "bar --option1 def"; + offset = parser.completeAdvanced(buffer, buffer.length(), candidates); + + assertThat(candidates, is(empty())); + } + @Test public void testDashDashIntoOption() { parser.add(new MyCommands()); @@ -206,6 +219,27 @@ public class SimpleParserTests { } + @Test + public void testWithDefaultKey() { + parser.add(new MyCommands()); + + buffer = "testDefaultKey thevalue --optio"; + offset = parser.completeAdvanced(buffer, buffer.length(), candidates); + + assertThat(candidates, hasItem(completionThat(is(equalTo("testDefaultKey thevalue --option2 "))))); + assertThat(candidates, hasItem(completionThat(is(equalTo("testDefaultKey thevalue --option3 "))))); + assertThat(candidates, not(hasItem(completionThat(containsString("option1"))))); + + // Let's do it again with default key in the middle + candidates.clear(); + buffer = "testDefaultKey --option3 foo thevalue --optio"; + offset = parser.completeAdvanced(buffer, buffer.length(), candidates); + + assertThat(candidates, hasItem(completionThat(is(equalTo("testDefaultKey --option3 foo thevalue --option2 "))))); + assertThat(candidates, not(hasItem(completionThat(endsWith("option3 "))))); + + } + @Test public void testStopAtFirstWhenMandatory() { parser.add(new MyCommands()); @@ -290,6 +324,14 @@ public class SimpleParserTests { String option3) { } + + @CliCommand("testDefaultKey") + public void testDefaultKey(@CliOption(key = "") + String option1, @CliOption(key = "option2") + String option2, @CliOption(key = "option3") + String option3) { + + } } public static class StringCompletions implements Converter { diff --git a/src/test/java/org/springframework/shell/core/TokenizerTests.java b/src/test/java/org/springframework/shell/core/TokenizerTests.java index 67355a4a..e8cae0f6 100644 --- a/src/test/java/org/springframework/shell/core/TokenizerTests.java +++ b/src/test/java/org/springframework/shell/core/TokenizerTests.java @@ -132,7 +132,7 @@ public class TokenizerTests { expected.put("foo", "bar bazz"); expected.put("bizz", "unfinished bizness "); assertEquals(expected, result); - assertTrue(tokenizer.lastValueIsStillBeingTyped()); + assertTrue(tokenizer.openingQuotesHaveNotBeenClosed()); } @Test @@ -143,7 +143,7 @@ public class TokenizerTests { expected.put("foo", "bar bazz"); expected.put("bizz", ""); assertEquals(expected, result); - assertTrue(tokenizer.lastValueIsStillBeingTyped()); + assertTrue(tokenizer.openingQuotesHaveNotBeenClosed()); } @Test