Add better support for modified option names

- Longnames in a command option if modified via
  name modifier didn't provide enough backmapping
  info for command execution experience being accurate.
- Add new getLongNamesModified() into CommandOption
  which is populated if name modifier is used.
- Add more hints in CommandExecution for modified
  option names.
- This should bring annotation
  and programmatic commands up to date.
- Backport #777
- Fixes #782
This commit is contained in:
Janne Valkealahti
2023-06-19 12:30:29 +01:00
parent 21fc4551e6
commit eaa363b34c
7 changed files with 68 additions and 16 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2022 the original author or authors.
* Copyright 2022-2023 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.
@@ -148,8 +148,9 @@ public interface CommandContext {
return results.results().stream()
.filter(r -> {
Stream<String> l = Arrays.asList(r.option().getLongNames()).stream();
Stream<String> lm = Arrays.asList(r.option().getLongNamesModified()).stream();
Stream<String> s = Arrays.asList(r.option().getShortNames()).stream().map(n -> Character.toString(n));
return Stream.concat(l, s)
return Stream.concat(Stream.concat(l, lm), s)
.filter(o -> ObjectUtils.nullSafeEquals(o, name))
.findFirst()
.isPresent();

View File

@@ -198,6 +198,11 @@ public interface CommandExecution {
messageBuilder.setHeader(ArgumentHeaderMethodArgumentResolver.ARGUMENT_PREFIX + n, r.value());
paramValues.put(n, r.value());
}
// need to provide backmapping for orinal names which were modified
for (String n : r.option().getLongNamesModified()) {
messageBuilder.setHeader(ArgumentHeaderMethodArgumentResolver.ARGUMENT_PREFIX + n, r.value());
paramValues.put(n, r.value());
}
}
if (r.option().getShortNames() != null) {
for (Character n : r.option().getShortNames()) {

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2022 the original author or authors.
* Copyright 2022-2023 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.
@@ -32,6 +32,14 @@ public interface CommandOption {
*/
String[] getLongNames();
/**
* Gets a modified long names of an option. Set within a command registration if
* option name modifier were used to have an info about original names.
*
* @return modified long names of an option
*/
String[] getLongNamesModified();
/**
* Gets a short names of an option.
*
@@ -111,7 +119,7 @@ public interface CommandOption {
* @return default command option
*/
public static CommandOption of(String[] longNames, Character[] shortNames, String description) {
return of(longNames, shortNames, description, null, false, null, null, null, null, null, null);
return of(longNames, null, shortNames, description, null, false, null, null, null, null, null, null);
}
/**
@@ -125,13 +133,14 @@ public interface CommandOption {
*/
public static CommandOption of(String[] longNames, Character[] shortNames, String description,
ResolvableType type) {
return of(longNames, shortNames, description, type, false, null, null, null, null, null, null);
return of(longNames, null, shortNames, description, type, false, null, null, null, null, null, null);
}
/**
* Gets an instance of a default {@link CommandOption}.
*
* @param longNames the long names
* @param longNamesModified the modified long names
* @param shortNames the short names
* @param description the description
* @param type the type
@@ -144,10 +153,10 @@ public interface CommandOption {
* @param completion the completion
* @return default command option
*/
public static CommandOption of(String[] longNames, Character[] shortNames, String description,
public static CommandOption of(String[] longNames, String[] longNamesModified, Character[] shortNames, String description,
ResolvableType type, boolean required, String defaultValue, Integer position, Integer arityMin,
Integer arityMax, String label, CompletionResolver completion) {
return new DefaultCommandOption(longNames, shortNames, description, type, required, defaultValue, position,
return new DefaultCommandOption(longNames, longNamesModified, shortNames, description, type, required, defaultValue, position,
arityMin, arityMax, label, completion);
}
@@ -157,6 +166,7 @@ public interface CommandOption {
public static class DefaultCommandOption implements CommandOption {
private String[] longNames;
private String[] longNamesModified;
private Character[] shortNames;
private String description;
private ResolvableType type;
@@ -168,11 +178,12 @@ public interface CommandOption {
private String label;
private CompletionResolver completion;
public DefaultCommandOption(String[] longNames, Character[] shortNames, String description,
public DefaultCommandOption(String[] longNames, String[] longNamesModified, Character[] shortNames, String description,
ResolvableType type, boolean required, String defaultValue, Integer position,
Integer arityMin, Integer arityMax, String label,
CompletionResolver completion) {
this.longNames = longNames != null ? longNames : new String[0];
this.longNamesModified = longNamesModified != null ? longNamesModified : new String[0];
this.shortNames = shortNames != null ? shortNames : new Character[0];
this.description = description;
this.type = type;
@@ -190,6 +201,11 @@ public interface CommandOption {
return longNames;
}
@Override
public String[] getLongNamesModified() {
return longNamesModified;
}
@Override
public Character[] getShortNames() {
return shortNames;

View File

@@ -1245,11 +1245,13 @@ public interface CommandRegistration {
options = optionSpecs.stream()
.map(o -> {
String[] longNames = o.getLongNames();
String[] longNamesModified = null;
Function<String, String> modifier = o.getOptionNameModifier();
if (modifier != null) {
longNamesModified = Arrays.copyOf(longNames, longNames.length);
longNames = Arrays.stream(longNames).map(modifier).toArray(String[]::new);
}
return CommandOption.of(longNames, o.getShortNames(), o.getDescription(), o.getType(),
return CommandOption.of(longNames, longNamesModified, o.getShortNames(), o.getDescription(), o.getType(),
o.isRequired(), o.getDefaultValue(), o.getPosition(), o.getArityMin(), o.getArityMax(),
o.getLabel(), o.getCompletion());
})

View File

@@ -539,4 +539,21 @@ public class CommandExecutionTests extends AbstractCommandTests {
Object result = execution.evaluate(r1, new String[]{"--arg1", "myarg1value"});
assertThat(result).isInstanceOf(CommandNotCurrentlyAvailable.class);
}
@Test
public void testExecutionWithModifiedLongOption() {
CommandRegistration r1 = CommandRegistration.builder()
.command("command1")
.withOption()
.longNames("arg1")
.nameModifier(orig -> "x" + orig)
.and()
.withTarget()
.function(function1)
.and()
.build();
Object result = execution.evaluate(r1, new String[] { "command1", "--xarg1", "myarg1value" });
assertThat(result).isEqualTo("himyarg1value");
}
}

View File

@@ -351,6 +351,7 @@ public class CommandParserTests extends AbstractCommandTests {
new String[] { "arg1" },
null,
null,
null,
ResolvableType.forType(int[].class),
true,
null,
@@ -385,6 +386,7 @@ public class CommandParserTests extends AbstractCommandTests {
new String[] { "arg1" },
null,
null,
null,
ResolvableType.forType(int[].class),
false,
null,
@@ -439,6 +441,7 @@ public class CommandParserTests extends AbstractCommandTests {
new String[] { "arg1" },
null,
null,
null,
ResolvableType.forType(List.class),
true,
null,
@@ -474,7 +477,7 @@ public class CommandParserTests extends AbstractCommandTests {
@Test
public void testBooleanWithDefault() {
ResolvableType type = ResolvableType.forType(boolean.class);
CommandOption option1 = CommandOption.of(new String[] { "arg1" }, new Character[0], "description", type, false,
CommandOption option1 = CommandOption.of(new String[] { "arg1" }, null, new Character[0], "description", type, false,
"true", null, null, null, null, null);
List<CommandOption> options = Arrays.asList(option1);
@@ -488,7 +491,7 @@ public class CommandParserTests extends AbstractCommandTests {
@Test
public void testIntegerWithDefault() {
ResolvableType type = ResolvableType.forType(Integer.class);
CommandOption option1 = CommandOption.of(new String[] { "arg1" }, new Character[0], "description", type, false,
CommandOption option1 = CommandOption.of(new String[] { "arg1" }, null, new Character[0], "description", type, false,
"1", null, null, null, null, null);
List<CommandOption> options = Arrays.asList(option1);
@@ -502,7 +505,7 @@ public class CommandParserTests extends AbstractCommandTests {
@Test
public void testIntegerWithGivenValue() {
ResolvableType type = ResolvableType.forType(Integer.class);
CommandOption option1 = CommandOption.of(new String[] { "arg1" }, new Character[0], "description", type, false,
CommandOption option1 = CommandOption.of(new String[] { "arg1" }, null, new Character[0], "description", type, false,
null, null, null, null, null, null);
List<CommandOption> options = Arrays.asList(option1);
@@ -586,6 +589,7 @@ public class CommandParserTests extends AbstractCommandTests {
null,
null,
null,
null,
true,
null,
0,
@@ -623,7 +627,7 @@ public class CommandParserTests extends AbstractCommandTests {
}
private static CommandOption longOption(String name, ResolvableType type, boolean required, Integer position, Integer arityMin, Integer arityMax) {
return CommandOption.of(new String[] { name }, new Character[0], "desc", type, required, null, position,
return CommandOption.of(new String[] { name }, null, new Character[0], "desc", type, required, null, position,
arityMin, arityMax, null, null);
}

View File

@@ -28,12 +28,13 @@ public class OptionNamingCommands {
public static class LegacyAnnotation extends BaseE2ECommands {
@ShellMethod(key = LEGACY_ANNO + "option-naming-1", group = GROUP)
public void testOptionNaming1Annotation(
public String testOptionNaming1Annotation(
@ShellOption("from_snake") String snake,
@ShellOption("fromCamel") String camel,
@ShellOption("from-kebab") String kebab,
@ShellOption("FromPascal") String pascal
) {
return String.format("snake='%s' camel='%s' kebab='%s' pascal='%s' ", snake, camel, kebab, pascal);
}
}
@@ -65,10 +66,16 @@ public class OptionNamingCommands {
.withOption()
.longNames("arg1")
.nameModifier(name -> "x" + name)
.required()
// .required()
.and()
.withTarget()
.consumer(ctx -> {})
.function(ctx -> {
String snake = ctx.getOptionValue("from_snake");
String camel = ctx.getOptionValue("fromCamel");
String kebab = ctx.getOptionValue("from-kebab");
String pascal = ctx.getOptionValue("FromPascal");
return String.format("snake='%s' camel='%s' kebab='%s' pascal='%s' ", snake, camel, kebab, pascal);
})
.and()
.build();
}