Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start of key value implementation #1144

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Start of key value implementation #1144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 56 additions & 3 deletions spring-shell-core/src/main/java/org/springframework/shell/command/parser/Lexer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,58 @@ public DefaultLexer(CommandModel commandModel, ParserConfig config) {
private record ArgumentsSplit(List<String> before, List<String> after) {
}


/**
* Loops over a list of arguments and if any of the arguments would follow the pattern ".*=.*"
* it splits those arguments.
* @param arguments the original arguments list
* @return List<String> containing the arguments without key-value separator
*/
private static List<String> splitKeyAndValueBySeparator(List<String> arguments) {
if(arguments != null){
if(arguments.isEmpty()){
return arguments;
}else{
List<String> result = new ArrayList<>();

for (String argument : arguments) {
if (argument == null) {
result.add(null);
} else if (argument.isEmpty()) {
result.add("");
} else if (argument.matches(".*=.*")) {
String[] parts = argument.split("=", 2);
result.add(parts[0].trim());
result.add(parts[1].trim());
} else {
result.add(argument); // Add non-key-value arguments as-is
}
}
return result;
}
}else{
return null;
}

}

/**
* Returns the argument split with the before and after list. In case isKeyValueSeparatorEnabled is true, it will however first separate the keys from values, adding
* each as an argument to the list so the tokenize method can work with this them as previously implemented.
* @param before before list
* @param after after list
* @param isKeyValueSeparatorEnabled true if key=value is allowed, otherwise not.
* @return ArgumentsSplit
*/
private ArgumentsSplit prepareArgumentSplitWithoutKeyValueSeparator(List<String> before, List<String> after, boolean isKeyValueSeparatorEnabled){
if(isKeyValueSeparatorEnabled){
return new ArgumentsSplit(splitKeyAndValueBySeparator(before), splitKeyAndValueBySeparator(after));
}else{
return new ArgumentsSplit(before, after);
}
}


/**
* Splits arguments from a point first valid command is found, where
* {@code before} is everything before commands and {@code after} what's
Expand All @@ -100,11 +152,12 @@ private ArgumentsSplit splitArguments(List<String> arguments, Map<String, Token>
}
else if (i == 0) {
if (foundSplit) {
return new ArgumentsSplit(Collections.emptyList(), arguments);
return prepareArgumentSplitWithoutKeyValueSeparator(Collections.emptyList(), arguments, config.isEnabled(Feature.ALLOW_KEY_VALUE_SEPARATOR));
}
return new ArgumentsSplit(arguments, Collections.emptyList());
return prepareArgumentSplitWithoutKeyValueSeparator(arguments, Collections.emptyList(), config.isEnabled(Feature.ALLOW_KEY_VALUE_SEPARATOR));
}
return new ArgumentsSplit(arguments.subList(0, i), arguments.subList(i, arguments.size()));

return prepareArgumentSplitWithoutKeyValueSeparator(arguments.subList(0, i), arguments.subList(i, arguments.size()), config.isEnabled(Feature.ALLOW_KEY_VALUE_SEPARATOR));
}

private List<String> extractDirectives(List<String> arguments) {
Expand Down
8 changes: 6 additions & 2 deletions spring-shell-core/src/main/java/org/springframework/shell/command/parser/ParserConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,12 @@ public static enum Feature {
/**
* Defines if options are parsed using case-sensitivity, enabled on default.
*/
CASE_SENSITIVE_OPTIONS(true)
;
CASE_SENSITIVE_OPTIONS(true),


ALLOW_KEY_VALUE_SEPARATOR(true);



private final boolean defaultState;
private final long mask;
Expand Down
176 changes: 176 additions & 0 deletions spring-shell-core/src/test/java/org/springframework/shell/command/parser/ParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,23 @@ void commandArgumentsGetsAddedAfterDoubleDash() {
}
);
}

@Test
void commandArgumentsGetsAddedAfterDoubleDashOptionValueSeparatedByEqualSign() {
register(ROOT3);
ParseResult result = parse("root3", "--arg1=value1", "--", "arg1", "arg2");

assertThat(result.argumentResults()).satisfiesExactly(
ar -> {
assertThat(ar.value()).isEqualTo("arg1");
assertThat(ar.position()).isEqualTo(0);
},
ar -> {
assertThat(ar.value()).isEqualTo("arg2");
assertThat(ar.position()).isEqualTo(1);
}
);
}
}

@Nested
Expand All @@ -95,6 +112,17 @@ void optionValueShouldBeInteger() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueShouldBeIntegerOptionValueSeparatedByEqualSign() {
register(ROOT6_OPTION_INT);
ParseResult result = parse("root6", "--arg1=1");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults().get(0).value()).isEqualTo(1);
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueShouldBeIntegerArray() {
register(ROOT6_OPTION_INTARRAY);
Expand All @@ -106,6 +134,18 @@ void optionValueShouldBeIntegerArray() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueShouldBeIntegerArrayOptionValueSeparatedByEqualSign() {
register(ROOT6_OPTION_INTARRAY);
//TODO: is this how we want to indicate arrays after the equal sign
ParseResult result = parse("root6", "--arg1=1", "2");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults().get(0).value()).isEqualTo(new int[] { 1, 2 });
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueFailsFromStringToInteger() {
register(ROOT6_OPTION_INT);
Expand All @@ -121,6 +161,21 @@ void optionValueFailsFromStringToInteger() {
});
}

@Test
void optionValueFailsFromStringToIntegerOptionValueSeparatedByEqualSign() {
register(ROOT6_OPTION_INT);
ParseResult result = parse("root6", "--arg1=x");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults().get(0).value()).isEqualTo("x");
assertThat(result.messageResults()).isNotEmpty();
assertThat(result.messageResults()).satisfiesExactly(ms -> {
// "2002E:(pos 0): Illegal option value 'x', reason 'Failed to convert from type [java.lang.String] to type [int] for value 'x''"
assertThat(ms.getMessage()).contains("Failed to convert");
});
}

@Test
void optionValueShouldBeNegativeInteger() {
register(ROOT6_OPTION_INT);
Expand All @@ -132,6 +187,17 @@ void optionValueShouldBeNegativeInteger() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueShouldBeNegativeIntegerOptionValueSeparatedByEqualSign() {
register(ROOT6_OPTION_INT);
ParseResult result = parse("root6", "--arg1=-1");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults().get(0).value()).isEqualTo(-1);
assertThat(result.messageResults()).isEmpty();
}

}

@Nested
Expand Down Expand Up @@ -312,6 +378,26 @@ void shouldFindTwoLongOptionArgument() {
);
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindTwoLongOptionArgumentOptionValueSeparatedByEqualSign() {
register(ROOT3_OPTION_ARG1_ARG2);
ParseResult result = parse("root3", "--arg1=value1", "--arg2=value2");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults()).satisfiesExactly(
r -> {
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg1" });
assertThat(r.value()).isEqualTo("value1");
},
r -> {
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg2" });
assertThat(r.value()).isEqualTo("value2");
}
);
assertThat(result.messageResults()).isEmpty();
}
}

@Nested
Expand Down Expand Up @@ -349,6 +435,22 @@ void shouldFindShortOptionWithArg() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptionWithOptionValueSeparatedByEqualSign() {
register(ROOT3_SHORT_OPTION_A);
ParseResult result = parse("root3", "-a=aaa");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults()).satisfiesExactly(
r -> {
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'a' });
assertThat(r.value()).isEqualTo("aaa");
}
);
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptions() {
register(ROOT3_SHORT_OPTION_A_B);
Expand All @@ -369,6 +471,26 @@ void shouldFindShortOptions() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptionsWithOptionValueSeparatedByEqualSign() {
register(ROOT3_SHORT_OPTION_A_B);
ParseResult result = parse("root3", "-a=aaa", "-b=bbb");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults()).satisfiesExactly(
r -> {
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'a' });
assertThat(r.value()).isEqualTo("aaa");
},
r -> {
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'b' });
assertThat(r.value()).isEqualTo("bbb");
}
);
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptionsRequired() {
register(ROOT3_SHORT_OPTION_A_B_REQUIRED);
Expand All @@ -379,6 +501,16 @@ void shouldFindShortOptionsRequired() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptionsRequiredWithOptionValueSeparatedByEqualSign() {
register(ROOT3_SHORT_OPTION_A_B_REQUIRED);
ParseResult result = parse("root3", "-a=aaa", "-b=bbb");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.messageResults()).isEmpty();
}

}

@Nested
Expand Down Expand Up @@ -439,6 +571,25 @@ void shouldFindLongOptionRegLowerCommandUpper() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindLongOptionRegLowerCommandUpperWithOptionValueSeparatedByEqualSign() {
register(ROOT3);
ParserConfig config = new ParserConfig()
.disable(Feature.CASE_SENSITIVE_COMMANDS)
.disable(Feature.CASE_SENSITIVE_OPTIONS)
;
ParseResult result = parse(config, "root3", "--Arg1=value1");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults()).satisfiesExactly(
r -> {
assertThat(r.value()).isEqualTo("value1");
}
);
assertThat(result.messageResults()).isEmpty();
}

}

@Nested
Expand Down Expand Up @@ -611,6 +762,31 @@ void positionOverridesDefaultsKeepsDefaultWhenOption() {
);
}

@Test
void positionOverridesDefaultsKeepsDefaultWhenOptionWithOptionValueSeparatedByEqualSign() {
register(ROOT7_POSITIONAL_TWO_ARG_STRING_DEFAULT);
ParseResult result = parse("root7", "--arg1=a", "b");
assertThat(result.messageResults()).isEmpty();
assertThat(result.argumentResults()).satisfiesExactly(
r -> {
assertThat(r.value()).isEqualTo("b");
assertThat(r.position()).isEqualTo(0);
}
);
assertThat(result.optionResults()).isNotNull().satisfiesExactly(
r -> {
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg1" });
assertThat(r.value()).isEqualTo("a");
},
r -> {
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg2" });
assertThat(r.value()).isEqualTo("b");
}
);
}



@Test
void positionWithLastHavingNoDefault() {
register(ROOT7_POSITIONAL_TWO_ARG_STRING_DEFAULT_ONE_NODEFAULT);
Expand Down