Skip to content

Commit d6ab2e5

Browse files
committed
Apply correct completion
- This commit fixes two issues. - Firstly complete with correct option as existing bug was to wrongly always complete with first option which used wrong provider. - Secondly filter out duplicate option proposals giving better result when options is already in place. - Backport #495 - Fixes #498 (cherry picked from commit a143d25)
1 parent 703ffc8 commit d6ab2e5

File tree

5 files changed

+220
-4
lines changed

5 files changed

+220
-4
lines changed

spring-shell-core/src/main/java/org/springframework/shell/Shell.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,8 @@ public List<CompletionProposal> complete(CompletionContext context) {
288288

289289
// Try to complete arguments
290290
List<CommandOption> matchedArgOptions = new ArrayList<>();
291-
if (argsContext.getWords().size() > 0) {
292-
matchedArgOptions.addAll(matchOptions(registration.getOptions(), argsContext.getWords().get(0)));
291+
if (argsContext.getWords().size() > 0 && argsContext.getWordIndex() > 0 && argsContext.getWords().size() > argsContext.getWordIndex()) {
292+
matchedArgOptions.addAll(matchOptions(registration.getOptions(), argsContext.getWords().get(argsContext.getWordIndex() - 1)));
293293
}
294294

295295
List<CompletionProposal> argProposals = matchedArgOptions.stream()

spring-shell-core/src/main/java/org/springframework/shell/completion/RegistrationOptionsCompletionResolver.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,15 @@ public List<CompletionProposal> apply(CompletionContext context) {
3838
List<CompletionProposal> candidates = new ArrayList<>();
3939
context.getCommandRegistration().getOptions().stream()
4040
.flatMap(o -> Stream.of(o.getLongNames()))
41-
.map(ln -> new CompletionProposal("--" + ln))
41+
.map(ln -> "--" + ln)
42+
.filter(ln -> !context.getWords().contains(ln))
43+
.map(CompletionProposal::new)
4244
.forEach(candidates::add);
4345
context.getCommandRegistration().getOptions().stream()
4446
.flatMap(o -> Stream.of(o.getShortNames()))
45-
.map(ln -> new CompletionProposal("-" + ln))
47+
.map(ln -> "-" + ln)
48+
.filter(ln -> !context.getWords().contains(ln))
49+
.map(CompletionProposal::new)
4650
.forEach(candidates::add);
4751
return candidates;
4852
}

spring-shell-core/src/test/java/org/springframework/shell/ShellTests.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,39 @@ public void completionArgWithFunction() throws Exception {
311311
assertThat(proposals).containsExactlyInAnyOrder("--arg1");
312312
}
313313

314+
@Test
315+
public void shouldCompleteWithCorrectArgument() throws Exception {
316+
CommandRegistration registration1 = CommandRegistration.builder()
317+
.command("hello world")
318+
.withTarget()
319+
.method(this, "helloWorld")
320+
.and()
321+
.withOption()
322+
.longNames("arg1")
323+
.completion(ctx -> Arrays.asList("arg1Comp1").stream().map(CompletionProposal::new).collect(Collectors.toList()))
324+
.and()
325+
.withOption()
326+
.longNames("arg2")
327+
.completion(ctx -> Arrays.asList("arg2Comp1").stream().map(CompletionProposal::new).collect(Collectors.toList()))
328+
.and()
329+
.build();
330+
Map<String, CommandRegistration> registrations = new HashMap<>();
331+
registrations.put("hello world", registration1);
332+
when(commandRegistry.getRegistrations()).thenReturn(registrations);
333+
334+
List<String> proposals1 = shell.complete(new CompletionContext(Arrays.asList("hello", "world", "--arg1", ""), 3, "".length(), null, null))
335+
.stream().map(CompletionProposal::value).collect(Collectors.toList());
336+
assertThat(proposals1).containsExactlyInAnyOrder("--arg2", "arg1Comp1");
337+
338+
List<String> proposals2 = shell.complete(new CompletionContext(Arrays.asList("hello", "world", "--arg1", "xxx", "--arg2", ""), 5, "".length(), null, null))
339+
.stream().map(CompletionProposal::value).collect(Collectors.toList());
340+
assertThat(proposals2).containsExactlyInAnyOrder("arg2Comp1");
341+
342+
List<String> proposals3 = shell.complete(new CompletionContext(Arrays.asList("hello", "world", "--arg2", "xxx", "--arg1", ""), 5, "".length(), null, null))
343+
.stream().map(CompletionProposal::value).collect(Collectors.toList());
344+
assertThat(proposals3).containsExactlyInAnyOrder("arg1Comp1");
345+
}
346+
314347
private static class Exit extends RuntimeException {
315348
}
316349

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
* Copyright 2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.shell.completion;
17+
18+
import java.util.Arrays;
19+
import java.util.List;
20+
21+
import org.junit.jupiter.api.Test;
22+
23+
import org.springframework.shell.CompletionContext;
24+
import org.springframework.shell.CompletionProposal;
25+
import org.springframework.shell.command.CommandRegistration;
26+
27+
import static org.assertj.core.api.Assertions.assertThat;
28+
29+
public class RegistrationOptionsCompletionResolverTests {
30+
31+
private final RegistrationOptionsCompletionResolver resolver = new RegistrationOptionsCompletionResolver();
32+
33+
@Test
34+
void completesAllOptions() {
35+
CommandRegistration registration = CommandRegistration.builder()
36+
.command("hello world")
37+
.withTarget()
38+
.consumer(ctx -> {})
39+
.and()
40+
.withOption()
41+
.longNames("arg1")
42+
.and()
43+
.withOption()
44+
.longNames("arg2")
45+
.and()
46+
.build();
47+
CompletionContext ctx = new CompletionContext(Arrays.asList("hello", "world", ""), 2, "".length(), registration, null);
48+
List<CompletionProposal> proposals = resolver.apply(ctx);
49+
assertThat(proposals).isNotNull();
50+
assertThat(proposals).hasSize(2);
51+
}
52+
53+
@Test
54+
void completesNonExistingOptions() {
55+
CommandRegistration registration = CommandRegistration.builder()
56+
.command("hello world")
57+
.withTarget()
58+
.consumer(ctx -> {})
59+
.and()
60+
.withOption()
61+
.longNames("arg1")
62+
.and()
63+
.withOption()
64+
.longNames("arg2")
65+
.and()
66+
.build();
67+
CompletionContext ctx = new CompletionContext(Arrays.asList("hello", "world", "--arg1", ""), 2, "".length(), registration, null);
68+
List<CompletionProposal> proposals = resolver.apply(ctx);
69+
assertThat(proposals).isNotNull();
70+
assertThat(proposals).hasSize(1);
71+
}
72+
73+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*
2+
* Copyright 2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.shell.samples.e2e;
17+
18+
import java.util.Arrays;
19+
import java.util.List;
20+
import java.util.stream.Collectors;
21+
22+
import org.springframework.context.annotation.Bean;
23+
import org.springframework.shell.CompletionContext;
24+
import org.springframework.shell.CompletionProposal;
25+
import org.springframework.shell.command.CommandRegistration;
26+
import org.springframework.shell.standard.ShellComponent;
27+
import org.springframework.shell.standard.ShellMethod;
28+
import org.springframework.shell.standard.ShellOption;
29+
import org.springframework.shell.standard.ValueProvider;
30+
31+
@ShellComponent
32+
public class InteractiveCompletionCommands extends BaseE2ECommands {
33+
34+
@ShellMethod(key = LEGACY_ANNO + "interactive-completion-1", group = GROUP)
35+
public String testInteractiveCompletion1(
36+
@ShellOption(valueProvider = Test1ValuesProvider.class) String arg1,
37+
@ShellOption(valueProvider = Test2ValuesProvider.class) String arg2
38+
) {
39+
return "Hello " + arg1;
40+
}
41+
42+
@Bean
43+
CommandRegistration testInteractiveCompletion1Registration() {
44+
Test1ValuesProvider test1ValuesProvider = new Test1ValuesProvider();
45+
Test2ValuesProvider test2ValuesProvider = new Test2ValuesProvider();
46+
return CommandRegistration.builder()
47+
.command(REG, "interactive-completion-1")
48+
.group(GROUP)
49+
.withOption()
50+
.longNames("arg1")
51+
.completion(ctx -> test1ValuesProvider.complete(ctx))
52+
.and()
53+
.withOption()
54+
.longNames("arg2")
55+
.completion(ctx -> test2ValuesProvider.complete(ctx))
56+
.and()
57+
.withTarget()
58+
.function(ctx -> {
59+
String arg1 = ctx.getOptionValue("arg1");
60+
return "Hello " + arg1;
61+
})
62+
.and()
63+
.build();
64+
}
65+
66+
@Bean
67+
Test1ValuesProvider test1ValuesProvider() {
68+
return new Test1ValuesProvider();
69+
}
70+
71+
@Bean
72+
Test2ValuesProvider test2ValuesProvider() {
73+
return new Test2ValuesProvider();
74+
}
75+
76+
static class Test1ValuesProvider implements ValueProvider {
77+
78+
private final static String[] VALUES = new String[] {
79+
"values1Complete1",
80+
"values1Complete2"
81+
};
82+
83+
@Override
84+
public List<CompletionProposal> complete(CompletionContext completionContext) {
85+
return Arrays.stream(VALUES)
86+
.map(CompletionProposal::new)
87+
.collect(Collectors.toList());
88+
}
89+
}
90+
91+
static class Test2ValuesProvider implements ValueProvider {
92+
93+
private final static String[] VALUES = new String[] {
94+
"values2Complete1",
95+
"values2Complete2"
96+
};
97+
98+
@Override
99+
public List<CompletionProposal> complete(CompletionContext completionContext) {
100+
return Arrays.stream(VALUES)
101+
.map(CompletionProposal::new)
102+
.collect(Collectors.toList());
103+
}
104+
}
105+
106+
}

0 commit comments

Comments
 (0)