From e0071e9abe2c21136afd926665f2b8f4acdc6a3f Mon Sep 17 00:00:00 2001 From: Eric Bottard Date: Mon, 14 Apr 2014 11:11:24 +0200 Subject: [PATCH] Revert "SHL-133 Shell command "! ls /tmp" fails" This reverts commit 497f60533ed140c64cb0ef0f0dda603f7b31e758. --- .../shell/commands/OsCommands.java | 2 - .../shell/commands/VersionCommands.java | 2 +- .../shell/core/SimpleParser.java | 15 +-- .../springframework/shell/core/Tokenizer.java | 92 ++++++------------- .../core/annotation/PassThroughOptions.java | 45 --------- .../shell/commands/BuiltInCommandTests.java | 16 ---- 6 files changed, 33 insertions(+), 139 deletions(-) delete mode 100644 src/main/java/org/springframework/shell/core/annotation/PassThroughOptions.java diff --git a/src/main/java/org/springframework/shell/commands/OsCommands.java b/src/main/java/org/springframework/shell/commands/OsCommands.java index e39ae138..67fcb5a7 100644 --- a/src/main/java/org/springframework/shell/commands/OsCommands.java +++ b/src/main/java/org/springframework/shell/commands/OsCommands.java @@ -21,7 +21,6 @@ import java.util.logging.Logger; import org.springframework.shell.core.CommandMarker; import org.springframework.shell.core.annotation.CliCommand; import org.springframework.shell.core.annotation.CliOption; -import org.springframework.shell.core.annotation.PassThroughOptions; import org.springframework.shell.support.logging.HandlerUtils; import org.springframework.stereotype.Component; @@ -40,7 +39,6 @@ public class OsCommands implements CommandMarker { private OsOperations osOperations = new OsOperationsImpl(); @CliCommand(value = "!", help = "Allows execution of operating system (OS) commands") - @PassThroughOptions public void command( @CliOption(key = { "", "command" }, mandatory = false, specifiedDefaultValue = "", unspecifiedDefaultValue = "", help = "The command to execute") final String command) { diff --git a/src/main/java/org/springframework/shell/commands/VersionCommands.java b/src/main/java/org/springframework/shell/commands/VersionCommands.java index 267ccd06..f9c70f24 100644 --- a/src/main/java/org/springframework/shell/commands/VersionCommands.java +++ b/src/main/java/org/springframework/shell/commands/VersionCommands.java @@ -1,4 +1,4 @@ - package org.springframework.shell.commands; +package org.springframework.shell.commands; import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; diff --git a/src/main/java/org/springframework/shell/core/SimpleParser.java b/src/main/java/org/springframework/shell/core/SimpleParser.java index ca9c0a04..c7905944 100644 --- a/src/main/java/org/springframework/shell/core/SimpleParser.java +++ b/src/main/java/org/springframework/shell/core/SimpleParser.java @@ -27,7 +27,6 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; @@ -36,7 +35,6 @@ import java.util.logging.Logger; import org.springframework.shell.core.annotation.CliAvailabilityIndicator; import org.springframework.shell.core.annotation.CliCommand; import org.springframework.shell.core.annotation.CliOption; -import org.springframework.shell.core.annotation.PassThroughOptions; import org.springframework.shell.event.ParseResult; import org.springframework.shell.support.logging.HandlerUtils; import org.springframework.shell.support.util.ExceptionUtils; @@ -159,19 +157,14 @@ public class SimpleParser implements Parser { // Attempt to parse Map options = null; - - // Hint to the parser that options will not be of the form --key=value, but should attempt to parse and - // pass through the command option 'as is' - PassThroughOptions passThroughOptions = methodTarget.getMethod() - .getAnnotation(PassThroughOptions.class); try { - options = new Tokenizer(methodTarget.getRemainingBuffer(), - false, passThroughOptions != null).getTokens(); - } catch (IllegalArgumentException e) { + options = new Tokenizer(methodTarget.getRemainingBuffer()).getTokens(); + } + catch (IllegalArgumentException e) { LOGGER.warning(ExceptionUtils.extractRootCause(e).getMessage()); return null; } - + final Set cliOptions = getCliOptions(parameterAnnotations); for (CliOption cliOption : cliOptions) { Class requiredType = methodTarget.getMethod().getParameterTypes()[arguments.size()]; diff --git a/src/main/java/org/springframework/shell/core/Tokenizer.java b/src/main/java/org/springframework/shell/core/Tokenizer.java index 8156a0ff..5cabe74a 100644 --- a/src/main/java/org/springframework/shell/core/Tokenizer.java +++ b/src/main/java/org/springframework/shell/core/Tokenizer.java @@ -28,16 +28,14 @@ import org.springframework.util.Assert; * Properly treats double quotes (") as option delimiters. * *

- * Expects option names to be preceded by a double dash. We call this an - * "option marker". + * Expects option names to be preceded by a double dash. We call this an "option marker". * *

* Treats spaces as the default option tokenizer. * *

- * 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. + * 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 @@ -56,14 +54,7 @@ public class Tokenizer { private boolean allowUnbalancedLastQuotedValue; /** - * When processing 'raw' command options, allow them to be aggregated - * together - */ - private boolean allowMultipleEmptyOptions; - - /** - * Used to indicate that the last value was indeed half enclosed in quotes. - * Useful so that parser can re-add it. + * Used to indicate that the last value was indeed half enclosed in quotes. Useful so that parser can re-add it. */ private boolean openingQuotesHaveNotBeenClosed; @@ -76,14 +67,8 @@ public class Tokenizer { } public Tokenizer(String text, boolean allowUnbalancedLastQuotedValue) { - this(text, allowUnbalancedLastQuotedValue, false); - } - - public Tokenizer(String text, boolean allowUnbalancedLastQuotedValue, - boolean allowMultipleEmptyOptions) { this.buffer = text.toCharArray(); this.allowUnbalancedLastQuotedValue = allowUnbalancedLastQuotedValue; - this.allowMultipleEmptyOptions = allowMultipleEmptyOptions; tokenize(); } @@ -107,8 +92,7 @@ public class Tokenizer { } /** - * Return true if the remaining buffer matches the given String (return - * false if there is not enough input). + * Return true if the remaining buffer matches the given String (return false if there is not enough input). */ private boolean lookAhead(char... toMatch) { for (int i = 0; i < toMatch.length; i++) { @@ -121,14 +105,14 @@ public class Tokenizer { } /** - * Consume either {@code --key[=value]} or just {@code value}, eating extra - * spaces. + * Consume either {@code --key[=value]} or just {@code value}, eating extra spaces. */ private void eatKeyValuePair() { if (lookAhead('-', '-')) { pos += 2; eatKeyEqualsValue(); - } else { + } + else { String value = eatValue(); store("", value); } @@ -136,18 +120,9 @@ public class Tokenizer { } private void store(String key, String value) { - if (this.allowMultipleEmptyOptions) { - //collect up all the options and reconstruct the full 'string' by appending - //each parsed option together with a space. - if (result.containsKey(key)) { - String originalValue = result.get(key); - result.put(key, originalValue + " " + value); - } else { - result.put(key, value); - } - } else if (result.put(key, value) != null) { - throw new IllegalArgumentException("You cannot specify option '" - + key + "' more than once in a single command"); + if (result.put(key, value) != null) { + throw new IllegalArgumentException("You cannot specify option '" + key + + "' more than once in a single command"); } } @@ -161,8 +136,7 @@ public class Tokenizer { endDelimiter = '"'; pos++; } - // So that it can be retrieved later (if this is actually the last - // value) + // So that it can be retrieved later (if this is actually the last value) lastValueDelimiter = endDelimiter; lastValueStartOffset = pos; while (pos < buffer.length && buffer[pos] != endDelimiter) { @@ -174,20 +148,18 @@ public class Tokenizer { sb.append(buffer[pos]); pos++; } - // When here, we either ran out of input, or encountered our delim, or - // both + // When here, we either ran out of input, or encountered our delim, or both // 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) + sb.length() == 0)) { // BUT it's ok if consumed nothing (pos-1 is *opening* quote then) if (allowUnbalancedLastQuotedValue) { openingQuotesHaveNotBeenClosed = true; return sb.toString(); - } else { - throw new IllegalArgumentException( - "Cannot have an unbalanced number of quotation marks"); + } + else { + throw new IllegalArgumentException("Cannot have an unbalanced number of quotation marks"); } } // Eat our delim @@ -196,36 +168,30 @@ public class Tokenizer { } /** - * Return the offset at which the last value seen started (NOT including any - * delimiter). + * Return the offset at which the last value seen started (NOT including any delimiter). */ public int getLastValueStartOffset() { - Assert.isTrue(lastValueStartOffset >= 0, - "lastValueStartOffset has not been set yet"); + Assert.isTrue(lastValueStartOffset >= 0, "lastValueStartOffset has not been set yet"); return lastValueStartOffset; } /** - * Return the delimiter (space or quotes) that was (or is being) used for - * the last value. + * Return the delimiter (space or quotes) that was (or is being) used for the last value. */ public char getLastValueDelimiter() { - Assert.isTrue(lastValueDelimiter != 0, - "lastValueDelimiter has not been set yet"); + Assert.isTrue(lastValueDelimiter != 0, "lastValueDelimiter has not been set yet"); return lastValueDelimiter; } /** - * Return whether the last value was meant to be enclosed in quotes, but the - * closing quote has not been typed yet. + * Return whether the last value was meant to be enclosed in quotes, but the closing quote has not been typed yet. */ public boolean openingQuotesHaveNotBeenClosed() { return openingQuotesHaveNotBeenClosed; } /** - * Return true if we know for sure that the last value has been typed in - * full. + * 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. @@ -234,12 +200,10 @@ public class Tokenizer { } /** - * Apply delimiter escaping to the given string, using the actual delimiter - * that was used for the last value. + * Apply delimiter escaping to the given string, using the actual delimiter that was used for the last value. */ public String escape(String value) { - return value.replace("" + lastValueDelimiter, "" + ESCAPE_CHAR - + lastValueDelimiter); + return value.replace("" + lastValueDelimiter, "" + ESCAPE_CHAR + lastValueDelimiter); } /** @@ -260,11 +224,11 @@ public class Tokenizer { lastValueDelimiter = ' '; lastValueStartOffset = pos; value = ""; - } else { + } + else { value = eatValue(); } - // Don't store the ""="" that would result from having a pending " --" - // at the end + // Don't store the ""="" that would result from having a pending " --" at the end if (key.equals("") && value.equals("")) { return; } diff --git a/src/main/java/org/springframework/shell/core/annotation/PassThroughOptions.java b/src/main/java/org/springframework/shell/core/annotation/PassThroughOptions.java deleted file mode 100644 index 90d6f314..00000000 --- a/src/main/java/org/springframework/shell/core/annotation/PassThroughOptions.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2011-2012 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.shell.core.annotation; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Inherited; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - - -/** - * A marker annotation on a method that indicates if the options for this command - * should not be parsed by the shell. - - * - *

- * An example of when to use this is when passing arguments through to the the underying - * Operating System Shell, e.g using the command " ! ls /tmp" * - *

- * - * @author Mark Pollack - * @since 1.1 - */ -@Inherited -@Documented -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.METHOD) -public @interface PassThroughOptions { - -} diff --git a/src/test/java/org/springframework/shell/commands/BuiltInCommandTests.java b/src/test/java/org/springframework/shell/commands/BuiltInCommandTests.java index b372eb27..5b9a7882 100644 --- a/src/test/java/org/springframework/shell/commands/BuiltInCommandTests.java +++ b/src/test/java/org/springframework/shell/commands/BuiltInCommandTests.java @@ -15,8 +15,6 @@ */ package org.springframework.shell.commands; -import static org.junit.Assert.assertTrue; - import java.text.DateFormat; import java.text.ParseException; import java.util.Date; @@ -45,18 +43,4 @@ public class BuiltInCommandTests extends AbstractShellIntegrationTest { Date now = new Date(); MatcherAssert.assertThat(now, DateMatchers.within(5, TimeUnit.SECONDS, result)); } - - @Test - public void OsCommandTest() throws ParseException { - - String osName = System.getProperty("os.name").toLowerCase(Locale.US); - String pathSep = System.getProperty("path.separator"); - boolean isUnix = pathSep.equals(":") && osName.endsWith("x"); - if (isUnix) { - CommandResult cr = getShell().executeCommand("! ls /tmp"); - assertTrue(cr.isSuccess()); - } - - } - }