diff --git a/src/main/java/org/springframework/shell/core/MethodTarget.java b/src/main/java/org/springframework/shell/core/MethodTarget.java index 8767e9a4..15d910a3 100644 --- a/src/main/java/org/springframework/shell/core/MethodTarget.java +++ b/src/main/java/org/springframework/shell/core/MethodTarget.java @@ -26,20 +26,23 @@ import org.springframework.util.StringUtils; * A method that can be executed via a shell command. *

* Immutable since 1.2.0. - * + * * @author Ben Alex */ public class MethodTarget { // Fields private final Method method; + private final Object target; + private final String remainingBuffer; + private final String key; /** * Constructor for a null remainingBuffer and key - * + * * @param method the method to invoke (required) * @param target the object on which the method is to be invoked (required) * @since 1.2.0 @@ -50,7 +53,7 @@ public class MethodTarget { /** * Constructor that allows all fields to be set - * + * * @param method the method to invoke (required) * @param target the object on which the method is to be invoked (required) * @param remainingBuffer can be blank @@ -62,7 +65,7 @@ public class MethodTarget { Assert.notNull(target, "Target is required"); this.key = StringUtils.trimWhitespace(key); this.method = method; - this.remainingBuffer = StringUtils.trimWhitespace(remainingBuffer); + this.remainingBuffer = remainingBuffer; this.target = target; } diff --git a/src/main/java/org/springframework/shell/core/SimpleParser.java b/src/main/java/org/springframework/shell/core/SimpleParser.java index 313022df..35be09ae 100644 --- a/src/main/java/org/springframework/shell/core/SimpleParser.java +++ b/src/main/java/org/springframework/shell/core/SimpleParser.java @@ -573,15 +573,8 @@ public class SimpleParser implements Parser { Assert.notNull(cmd, "CliCommand unavailable for '" + methodTarget.getMethod().toGenericString() + "'"); // Make a reasonable attempt at parsing the remainingBuffer - Map options; - try { - options = new Tokenizer(methodTarget.getRemainingBuffer()).getTokens(); - } - catch (IllegalArgumentException ex) { - // Assume any IllegalArgumentException is due to a quotation mark mismatch - candidates.add(new Completion(translated + "\"")); - return 0; - } + Tokenizer tokenizer = new Tokenizer(methodTarget.getRemainingBuffer(), true); + Map options = tokenizer.getTokens(); // Lookup arguments for this target Annotation[][] parameterAnnotations = methodTarget.getMethod().getParameterAnnotations(); @@ -665,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("--")) { + if (methodTarget.getRemainingBuffer().endsWith("--") && !tokenizer.lastValueHadQuote()) { boolean showAllRemaining = true; for (CliOption include : unspecified) { if (include.mandatory()) { @@ -691,7 +684,8 @@ public class SimpleParser implements Parser { // Handle suggesting an option key if they haven't got one presently specified (or they've completed a full // option key/value pair) if (lastOptionKey == null - || (!"".equals(lastOptionKey) && !"".equals(lastOptionValue) && translated.endsWith(" "))) { + || (!"".equals(lastOptionKey) && !"".equals(lastOptionValue) && translated.endsWith(" ") && !tokenizer + .lastValueHadQuote())) { // We have either NEVER specified an option key/value pair // OR we have specified a full option key/value pair @@ -755,7 +749,8 @@ public class SimpleParser implements Parser { } // Handle completing the option key they're presently typing - if ((lastOptionValue == null || "".equals(lastOptionValue)) && !translated.endsWith(" ")) { + if ((lastOptionValue == null || "".equals(lastOptionValue)) + && !(translated.endsWith(" ") || translated.endsWith(" \""))) { // Given we haven't got an option value of any form, and there's no space at the buffer end, we must // still be typing an option key // System.out.println("completing an option"); @@ -827,9 +822,13 @@ public class SimpleParser implements Parser { } String prefix = ""; - if (!translated.endsWith(" ")) { + if (!tokenizer.lastValueHadQuote() && !translated.endsWith(" ")) { prefix = " "; } + else if (tokenizer.lastValueHadQuote()) { + // Re-install opening quote if there was one + prefix = " \""; + } // Only include in the candidates those results which are compatible with the present buffer for (Completion currentValue : allValues) { @@ -896,11 +895,12 @@ public class SimpleParser implements Parser { if (results.size() > 0) { candidates.addAll(results); - // Values presented from the last space onwards - if (translated.endsWith(" ")) { + if (tokenizer.lastValueHadQuote()) { + return translated.lastIndexOf(" \""); + } + else { return translated.lastIndexOf(" ") + 1; } - return translated.trim().lastIndexOf(" "); } return 0; } diff --git a/src/main/java/org/springframework/shell/core/Tokenizer.java b/src/main/java/org/springframework/shell/core/Tokenizer.java index a7e0ffd8..8fcd4cd3 100644 --- a/src/main/java/org/springframework/shell/core/Tokenizer.java +++ b/src/main/java/org/springframework/shell/core/Tokenizer.java @@ -35,6 +35,8 @@ import java.util.Map; * Any token without an option marker is considered the default. The default is returned in the Map as an element with * an empty string key (""). There can only be a single default. * + * @author Eric Bottard + * @since 1.1 */ public class Tokenizer { @@ -46,8 +48,21 @@ public class Tokenizer { private final Map result = new LinkedHashMap(); + /** Useful when trying to do auto complete. */ + private boolean allowUnbalancedLastQuotedValue; + + /** + * Used to indicate that the last value was indeed half enclosed in quotes. Useful so that parser can re-add it. + */ + private boolean lastValueHadQuote; + public Tokenizer(String text) { + this(text, false); + } + + public Tokenizer(String text, boolean allowUnbalancedLastQuotedValue) { this.buffer = text.toCharArray(); + this.allowUnbalancedLastQuotedValue = allowUnbalancedLastQuotedValue; tokenize(); } @@ -109,19 +124,46 @@ public class Tokenizer { pos++; } // When here, we either ran out of input, or encountered our delim, or both - if (endDelimiter == '"' && pos == buffer.length && buffer[pos - 1] != '"') { - throw new IllegalArgumentException("Cannot have an unbalanced number of quotation marks"); + // Fail, unless we allow an unfinished quoted string to be reported + if (endDelimiter == '"' && // we're using quotes + pos == buffer.length && // we ran of input + (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) { + lastValueHadQuote = true; + return sb.toString(); + } + else { + throw new IllegalArgumentException("Cannot have an unbalanced number of quotation marks"); + } } // Eat our delim pos++; return sb.toString(); } + public boolean lastValueHadQuote() { + return lastValueHadQuote; + } + + /** + * Consume a full @code{--key value} pair *unless* we're at the very end, in which case allow for @code {--key}, + * using "" for the value. + */ private void eatKeyEqualsValue() { String key = eatKey(); eatWhiteSpace(); - String value = eatValue(); - store(key, value); + String value; + if (pos < buffer.length) { + value = eatValue(); + } + else { + value = ""; + } + if (!key.equals("") || !key.equals(value)) { + // Don't store the ""="" that would result from having a pending " --" at the end + store(key, value); + } } private String eatKey() { diff --git a/src/test/java/org/springframework/shell/core/TokenizerTests.java b/src/test/java/org/springframework/shell/core/TokenizerTests.java index 2878d429..ff369262 100644 --- a/src/test/java/org/springframework/shell/core/TokenizerTests.java +++ b/src/test/java/org/springframework/shell/core/TokenizerTests.java @@ -19,6 +19,7 @@ package org.springframework.shell.core; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.util.HashMap; import java.util.Map; @@ -112,6 +113,45 @@ public class TokenizerTests { assertEquals(singletonMap("foo", "bar \"fizz"), result); } + @Test + public void testAllowKeyOnlyIfAtTheEnd() { + Map result = tokenize("--foo bar fizz --bozz "); + Map expected = new HashMap(); + expected.put("", "fizz"); + expected.put("foo", "bar"); + expected.put("bozz", ""); + assertEquals(expected, result); + + } + + @Test + public void testValueQuotationAllowUnfinished() { + Tokenizer tokenizer = new Tokenizer("--foo \"bar bazz\" --bizz \"unfinished bizness ", true); + Map result = tokenizer.getTokens(); + Map expected = new HashMap(); + expected.put("foo", "bar bazz"); + expected.put("bizz", "unfinished bizness "); + assertEquals(expected, result); + assertTrue(tokenizer.lastValueHadQuote()); + } + + @Test + public void testValueQuotationAllowUnfinishedEvenWithEmptyContent() { + Tokenizer tokenizer = new Tokenizer("--foo \"bar bazz\" --bizz \"", true); + Map result = tokenizer.getTokens(); + Map expected = new HashMap(); + expected.put("foo", "bar bazz"); + expected.put("bizz", ""); + assertEquals(expected, result); + assertTrue(tokenizer.lastValueHadQuote()); + } + + @Test + public void testPendingDashDash() { + Map result = tokenize("--foo bar --"); + assertEquals(singletonMap("foo", "bar"), result); + } + private Map tokenize(String what) { return new Tokenizer(what).getTokens(); }