Skip to content

Commit 6e1ca08

Browse files
committed
Replace parser string joining with list
- Issue in #622 is that its command type is `String` and internally some incoming arguments(it's List) were converted to String by joining with space. This caused one case with help command to get conversion via spring's ConversionService(CollectionToStringConverter) which joins by commas. That was we saw in failed example. - Remove needed joins in CommandParser and let it just pass List which then works better with ConversionService. - This then needs a `command` option type change from String to String[] which it really is as you should be able to give whole command as an argument. - Fixes #622
1 parent a04091c commit 6e1ca08

File tree

4 files changed

+29
-16
lines changed

4 files changed

+29
-16
lines changed

spring-shell-core/src/main/java/org/springframework/shell/command/CommandParser.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ public CommandParserResults parse(List<CommandOption> options, String[] args) {
261261
}
262262
if (pop != null && pop.option == null) {
263263
if (!pop.args.isEmpty()) {
264-
oargs.add(pop.args.stream().collect(Collectors.joining(" ")));
264+
oargs.addAll(pop.args);
265265
}
266266
}
267267
}
@@ -370,11 +370,7 @@ ParserResults visit(List<List<String>> lexerResults, List<CommandOption> options
370370
.filter(co -> co.getDefaultValue() != null)
371371
.forEach(co -> {
372372
Object value = co.getDefaultValue();
373-
if (conversionService != null && co.getType() != null) {
374-
if (conversionService.canConvert(co.getDefaultValue().getClass(), co.getType().getRawClass())) {
375-
value = conversionService.convert(co.getDefaultValue(), co.getType().getRawClass());
376-
}
377-
}
373+
value = convertOptionType(co, value);
378374
results.add(ParserResult.of(co, Collections.emptyList(), value, null));
379375
});
380376
return ParserResults.of(results);
@@ -478,7 +474,7 @@ else if (type != null && type.isArray()) {
478474
else {
479475
if (arityMax > 0) {
480476
int limit = Math.min(arguments.size(), arityMax);
481-
value = arguments.stream().limit(limit).collect(Collectors.joining(" "));
477+
value = arguments.stream().limit(limit).collect(Collectors.toList());
482478
unmapped.addAll(arguments.subList(limit, arguments.size()));
483479
}
484480
else {

spring-shell-core/src/test/java/org/springframework/shell/command/CommandParserTests.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ public void testMultipleArgsWithMultiValues() {
9494
CommandParserResults results = parser.parse(options, args);
9595
assertThat(results.results()).hasSize(2);
9696
assertThat(results.results().get(0).option()).isSameAs(option1);
97-
assertThat(results.results().get(0).value()).isEqualTo("foo1 foo2");
97+
assertThat(results.results().get(0).value()).isEqualTo(Arrays.asList("foo1", "foo2"));
9898
assertThat(results.results().get(1).option()).isSameAs(option2);
99-
assertThat(results.results().get(1).value()).isEqualTo("bar1 bar2");
99+
assertThat(results.results().get(1).value()).isEqualTo(Arrays.asList("bar1", "bar2"));
100100
assertThat(results.positional()).isEmpty();
101101
}
102102

@@ -215,6 +215,18 @@ public void testNonMappedArgWithoutOptionHavingType() {
215215
assertThat(results.positional()).containsExactly("value", "foo");
216216
}
217217

218+
@Test
219+
public void testMappedFromArgToString() {
220+
CommandOption option1 = longOption("arg1", ResolvableType.forType(String.class), false, 0, 1, 2);
221+
List<CommandOption> options = Arrays.asList(option1);
222+
String[] args = new String[]{"--arg1", "value", "foo"};
223+
CommandParserResults results = parser.parse(options, args);
224+
assertThat(results.results()).hasSize(1);
225+
assertThat(results.results().get(0).option()).isSameAs(option1);
226+
assertThat(results.results().get(0).value()).isEqualTo("value,foo");
227+
assertThat(results.positional()).isEmpty();
228+
}
229+
218230
@Test
219231
public void testShortOptionsCombined() {
220232
CommandOption optionA = shortOption('a');
@@ -381,7 +393,7 @@ public void testMapPositionalArgs11() {
381393
assertThat(results.results()).hasSize(2);
382394
assertThat(results.results().get(0).option()).isSameAs(option1);
383395
assertThat(results.results().get(1).option()).isSameAs(option2);
384-
assertThat(results.results().get(0).value()).isEqualTo("1");
396+
assertThat(results.results().get(0).value()).isEqualTo(Arrays.asList("1"));
385397
// no type so we get raw list
386398
assertThat(results.results().get(1).value()).isEqualTo(Arrays.asList("2"));
387399
}

spring-shell-standard-commands/src/main/java/org/springframework/shell/standard/commands/Help.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2017-2022 the original author or authors.
2+
* Copyright 2017-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,6 +22,8 @@
2222
import java.nio.charset.StandardCharsets;
2323
import java.util.HashMap;
2424
import java.util.Map;
25+
import java.util.stream.Collectors;
26+
import java.util.stream.Stream;
2527

2628
import org.jline.utils.AttributedString;
2729

@@ -76,13 +78,16 @@ public Help(TemplateExecutor templateExecutor) {
7678
@ShellMethod(value = "Display help about available commands")
7779
public AttributedString help(
7880
@ShellOption(defaultValue = ShellOption.NULL, valueProvider = CommandValueProvider.class, value = { "-C",
79-
"--command" }, help = "The command to obtain help for.", arity = Integer.MAX_VALUE) String command)
81+
"--command" }, help = "The command to obtain help for.", arity = Integer.MAX_VALUE) String[] command)
8082
throws IOException {
8183
if (command == null) {
8284
return renderCommands();
8385
}
8486
else {
85-
return renderCommand(command);
87+
String commandStr = Stream.of(command)
88+
.map(c -> c.trim())
89+
.collect(Collectors.joining(" "));
90+
return renderCommand(commandStr);
8691
}
8792
}
8893

spring-shell-standard-commands/src/test/java/org/springframework/shell/standard/commands/HelpTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022 the original author or authors.
2+
* Copyright 2022-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -121,7 +121,7 @@ public void testCommandHelp() throws Exception {
121121
.and()
122122
.build();
123123
commandCatalog.register(registration);
124-
String help = this.help.help("first-command").toString();
124+
String help = this.help.help(new String[] { "first-command" }).toString();
125125
help = removeNewLines(help);
126126
assertThat(help).isEqualTo(sample());
127127
}
@@ -146,7 +146,7 @@ public void testCommandListFlat() throws Exception {
146146
@Test
147147
public void testUnknownCommand() throws Exception {
148148
assertThatThrownBy(() -> {
149-
this.help.help("some unknown command");
149+
this.help.help(new String[] { "some", "unknown", "command" });
150150
}).isInstanceOf(IllegalArgumentException.class);
151151
}
152152

0 commit comments

Comments
 (0)