From 295b3d3becbc19354c52e9b1c4de422991e04444 Mon Sep 17 00:00:00 2001 From: Ruud Senden Date: Mon, 13 Jun 2022 16:28:58 +0200 Subject: [PATCH] [https://github.com/remkop/picocli/issues/1706] Resolve messages against parent resource bundles with updated i18n unit tests and fixed whitespace. --- src/main/java/picocli/CommandLine.java | 51 +++++++++--- src/test/java/picocli/I18nCommand.java | 10 ++- src/test/java/picocli/I18nSubcommand2.java | 30 +++++++ src/test/java/picocli/I18nTest.java | 80 +++++++++++++++++-- .../ResourceBundlePropagationTest.properties | 3 +- .../picocli/SharedMessages.properties | 3 + .../picocli/SharedMessages_ja.properties | 1 + 7 files changed, 159 insertions(+), 19 deletions(-) create mode 100644 src/test/java/picocli/I18nSubcommand2.java diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java index 2d370729a..08df0c1c4 100644 --- a/src/main/java/picocli/CommandLine.java +++ b/src/main/java/picocli/CommandLine.java @@ -40,6 +40,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Matcher; import java.util.regex.Pattern; + import picocli.CommandLine.Help.Ansi.IStyle; import picocli.CommandLine.Help.Ansi.Style; import picocli.CommandLine.Help.Ansi.Text; @@ -11559,6 +11560,7 @@ public static class Messages { private final String bundleBaseName; private final ResourceBundle rb; private final Set keys; + private Messages parent; public Messages(CommandSpec spec, String baseName) { this(spec, baseName, createBundle(baseName)); } @@ -11582,6 +11584,21 @@ private static String extractName(ResourceBundle rb) { return (String) ResourceBundle.class.getDeclaredMethod("getBaseBundleName").invoke(rb); } catch (Exception ignored) { return "?"; } } + public Messages parent() { + CommandSpec parentSpec = this.spec.parent(); + if (parent == null || parent.spec != parentSpec) { + parent = null; // Refresh if parentSpec doesn't match + while (parent == null && parentSpec != null) { + String parentResourceBundleBaseName = parentSpec.resourceBundleBaseName(); + if (parentResourceBundleBaseName != null && !parentResourceBundleBaseName.equals(this.bundleBaseName)) { + parent = new Messages(parentSpec, parentResourceBundleBaseName); + } else { + parentSpec = parentSpec.parent(); + } + } + } + return parent; + } private static Set keys(ResourceBundle rb) { if (rb == null) { return Collections.emptySet(); } Set keys = new LinkedHashSet(); @@ -11597,8 +11614,8 @@ private static Set keys(ResourceBundle rb) { public static Messages copy(CommandSpec spec, Messages original) { return original == null ? null : new Messages(spec, original.bundleBaseName, original.rb); } - /** Returns {@code true} if the specified {@code Messages} is {@code null} or has a {@code null ResourceBundle}. */ - public static boolean empty(Messages messages) { return messages == null || messages.rb == null; } + /** Returns {@code true} if the specified {@code Messages} is {@code null}, has a {@code null ResourceBundle}, or has a {@code null parent Messages}. */ + public static boolean empty(Messages messages) { return messages == null || messages.isEmpty(); } /** Returns the String value found in the resource bundle for the specified key, or the specified default value if not found. * @param key unqualified resource bundle key. This method will first try to find a value by qualifying the key with the command's fully qualified name, @@ -11609,12 +11626,19 @@ public static Messages copy(CommandSpec spec, Messages original) { public String getString(String key, String defaultValue) { if (isEmpty()) { return defaultValue; } String cmd = spec.qualifiedName("."); - if (keys.contains(cmd + "." + key)) { return rb.getString(cmd + "." + key); } - if (keys.contains(key)) { return rb.getString(key); } - return defaultValue; + String qualifiedKey = cmd + "." + key; + String result = getStringForExactKey(qualifiedKey); + if (result == null) { result = getStringForExactKey(key); } + return result != null ? result : defaultValue; } - boolean isEmpty() { return rb == null || keys.isEmpty(); } + private String getStringForExactKey(String key) { + if (keys.contains(key)) { return rb.getString(key); } + else if (parent() != null) { return parent().getStringForExactKey(key); } + else { return null; } + } + + boolean isEmpty() { return (rb == null || keys.isEmpty()) && (parent() == null || parent().isEmpty()); } /** Returns the String array value found in the resource bundle for the specified key, or the specified default value if not found. * Multi-line strings can be specified in the resource bundle with {@code key.0}, {@code key.1}, {@code key.2}, etc. @@ -11626,10 +11650,15 @@ public String getString(String key, String defaultValue) { public String[] getStringArray(String key, String[] defaultValues) { if (isEmpty()) { return defaultValues; } String cmd = spec.qualifiedName("."); - List result = addAllWithPrefix(rb, cmd + "." + key, keys, new ArrayList()); - if (!result.isEmpty()) { return result.toArray(new String[0]); } - addAllWithPrefix(rb, key, keys, result); - return result.isEmpty() ? defaultValues : result.toArray(new String[0]); + String qualifiedKey = cmd + "." + key; + String[] result = getStringArrayForExactKey(qualifiedKey); + if (result == null) { result = getStringArrayForExactKey(key); } + return result != null ? result : defaultValues; + } + private String[] getStringArrayForExactKey(String key) { + List result = addAllWithPrefix(rb, key, keys, new ArrayList()); + if (!result.isEmpty()) { return result.toArray(new String[0]); } + return parent() == null ? null : parent().getStringArrayForExactKey(key); } private static List addAllWithPrefix(ResourceBundle rb, String key, Set keys, List result) { if (keys.contains(key)) { result.add(rb.getString(key)); } @@ -18237,7 +18266,7 @@ private Tracer() {} /** Returns whether the current trace level is WARN or higher. */ public boolean isWarn() { return level.isEnabled(TraceLevel.WARN); } /** Returns whether the current trace level is OFF (the lowest). */ - public boolean isOff() { return level== TraceLevel.OFF; } + public boolean isOff() { return level == TraceLevel.OFF; } /** Prints the specified message if the current trace level is WARN or higher. * @param msg the message to print; may use {@link String#format(String, Object...)} syntax * @param params Arguments referenced by the format specifiers in the format string. If there are more arguments than format specifiers, the extra arguments are ignored. The number of arguments is variable and may be zero. diff --git a/src/test/java/picocli/I18nCommand.java b/src/test/java/picocli/I18nCommand.java index 03de13218..32ef72a50 100644 --- a/src/test/java/picocli/I18nCommand.java +++ b/src/test/java/picocli/I18nCommand.java @@ -3,10 +3,11 @@ import picocli.CommandLine.Command; import picocli.CommandLine.Option; import picocli.CommandLine.Parameters; +import picocli.CommandLine.Mixin; @Command(name = "i18n-top", resourceBundle = "picocli.SharedMessages", - subcommands = {CommandLine.HelpCommand.class, I18nSubcommand.class }, + subcommands = {CommandLine.HelpCommand.class, I18nSubcommand.class, I18nSubcommand2.class }, mixinStandardHelpOptions = true, description = {"top desc 1", "top desc 2", "top desc 3"}, descriptionHeading = "top desc heading%n", @@ -33,6 +34,13 @@ public class I18nCommand { @Parameters(index = "1", description = "top param1 description") String param1; + @Option( + names = {"--optionWithDescriptionFromParent"}, + descriptionKey = "optionWithDescriptionFromParent", + scope = CommandLine.ScopeType.INHERIT + ) + public boolean parentOption1; + @Override public String toString() { return getClass().getName(); diff --git a/src/test/java/picocli/I18nSubcommand2.java b/src/test/java/picocli/I18nSubcommand2.java new file mode 100644 index 000000000..c10fc188b --- /dev/null +++ b/src/test/java/picocli/I18nSubcommand2.java @@ -0,0 +1,30 @@ +package picocli; + +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; +import picocli.CommandLine.Mixin; + +/** + * This command was created to test Issue #1706. + * We are testing here to ensure that the description for a shared Mixin option, where the description for that option + * is defined in a parent command's resource bundle, can still be displayed correctly by a subcommand that's using a + * resource bundle that doesn't have that Mixin's option description defined. Ideally, if the subcommand's resource + * bundle doesn't have the needed description key, picocli SHOULD check the resource bundle of the parent command to see + * if the description key exists. + */ +@Command(name = "sub2", + subcommands = CommandLine.HelpCommand.class, + mixinStandardHelpOptions = true, + resourceBundle = "picocli.ResourceBundlePropagationTest") +public class I18nSubcommand2 { + @Option(names = {"-a", "--aaa"}, descriptionKey = "additional.value") + String a; + + // There's an option, --optionWithDescriptionFromParent, which is inherited from the parent command with description + + @Option(names = {"-x"}) // Description should come from Parent command's resource bundle + String x; + + @Option(names = {"-q"}) // Description is in both parent & sub command resource bundles, but subcommand's should take priority + String q; +} diff --git a/src/test/java/picocli/I18nTest.java b/src/test/java/picocli/I18nTest.java index 5526fb7b1..f1265eaf1 100644 --- a/src/test/java/picocli/I18nTest.java +++ b/src/test/java/picocli/I18nTest.java @@ -184,7 +184,8 @@ public void testParentCommandWithSharedResourceBundle() { "i18n-top header heading%n" + "Shared header first line%n" + "Shared header second line%n" + - "Usage: i18n-top [-hV] [-x=] [-y=] [-z=] [COMMAND]%n" + + "Usage: i18n-top [-hV] [--optionWithDescriptionFromParent] [-x=] [-y=]%n" + + " [-z=] [COMMAND]%n" + "i18n-top description heading:%n" + "Shared description 0%n" + "Shared description 1%n" + @@ -195,6 +196,9 @@ public void testParentCommandWithSharedResourceBundle() { " shared param1 desc line 1%n" + "top option list heading%n" + " -h, --help Shared help option description.%n" + + " --optionWithDescriptionFromParent%n" + + " This description should be visible in both parent and sub%n" + + " commands%n" + " -V, --version Print version information and exit.%n" + " -x, --xxx= X option description for i18n-top%n" + " -y, --yyy= top yyy description 1%n" + @@ -203,6 +207,7 @@ public void testParentCommandWithSharedResourceBundle() { "top command list heading%n" + " help i18n-top HELP command header%n" + " i18n-sub i18n-sub header (only one line)%n" + + " sub2 Shared header first line%n" + "Shared Exit Codes Heading%n" + "These exit codes are blah blah etc.%n" + " 00 (From shared bundle) Normal termination%n" + @@ -215,6 +220,7 @@ public void testParentCommandWithSharedResourceBundle() { Locale original = Locale.getDefault(); Locale.setDefault(Locale.ENGLISH); try { + String msg = new CommandLine(new I18nCommand()).getUsageMessage(); assertEquals(expected, new CommandLine(new I18nCommand()).getUsageMessage()); } finally { Locale.setDefault(original); @@ -227,7 +233,8 @@ public void testParentCommandWithSharedResourceBundle_ja() { "i18n-top \u30d8\u30c3\u30c0\u30fc\u898b\u51fa\u3057%n" + "\u5171\u901a\u30d8\u30c3\u30c0\u30fc\uff11\u884c\u76ee%n" + "\u5171\u901a\u30d8\u30c3\u30c0\u30fc\uff12\u884c\u76ee%n" + - "Usage: i18n-top [-hV] [-x=] [-y=] [-z=] [COMMAND]%n" + + "Usage: i18n-top [-hV] [--optionWithDescriptionFromParent] [-x=] [-y=]%n" + + " [-z=] [COMMAND]%n" + "i18n-top \u8aac\u660e\u898b\u51fa\u3057:%n" + "\u5171\u901a\u8aac\u660e0%n" + "\u5171\u901a\u8aac\u660e1%n" + @@ -238,6 +245,9 @@ public void testParentCommandWithSharedResourceBundle_ja() { " \u5171\u901a param1 \u8aac\u660e\uff12\u884c\u76ee%n" + "top option list heading%n" + " -h, --help \u5171\u901a help \u30aa\u30d7\u30b7\u30e7\u30f3\u8aac\u660e.%n" + + " --optionWithDescriptionFromParent%n" + + " \u3053\u306e\u8aac\u660e\u306f\u3001\u89aa\u30b3\u30de\u30f3\u30c9\u3068\u30b5\u30d6\u30b3\u30de\u30f3\u30c9\u306e\u4e21\u65b9\u306b\u8868\u793a\u3055\u308c\u308b\u5fc5\u8981\u304c\u3042%n" + + " \u308a\u307e\u3059%n" + " -V, --version Print version information and exit.%n" + " -x, --xxx= i18n-top\u7528 X \u30aa\u30d7\u30b7\u30e7\u30f3\u8aac\u660e%n" + " -y, --yyy= top yyy description 1%n" + @@ -246,6 +256,7 @@ public void testParentCommandWithSharedResourceBundle_ja() { "top command list heading%n" + " help i18n-top\u7528 HELP \u30b3\u30de\u30f3\u30c9\u30d8\u30c3\u30c0\u30fc%n" + " i18n-sub i18n-sub \u30d8\u30c3\u30c0\u30fc\uff08\u4e00\u884c\u306e\u307f\uff09%n" + + " sub2 \u5171\u901a\u30d8\u30c3\u30c0\u30fc\uff11\u884c\u76ee%n" + "\u7d42\u4e86\u30b9\u30c6\u30fc\u30bf\u30b9%n" + "\u3053\u308c\u3089\u306e\u7d42\u4e86\u30b9\u30c6\u30fc\u30bf\u30b9\u306f\u7b49\u3005%n" + " 00 (\u5171\u901a\u30d0\u30f3\u30c9\u30eb\u304b\u3089) \u6b63\u5e38\u7d42\u4e86%n" + @@ -269,8 +280,8 @@ public void testSubcommandUsesParentBundle() { String expected = String.format("" + "i18n-sub header heading%n" + "i18n-sub header (only one line)%n" + - "Usage: i18n-top i18n-sub [-hV] [-x=] [-y=] [-z=] %n" + - " [COMMAND]%n" + + "Usage: i18n-top i18n-sub [-hV] [--optionWithDescriptionFromParent] [-x=]%n" + + " [-y=] [-z=] [COMMAND]%n" + "i18n-sub description heading:%n" + "Shared description 0%n" + "Shared description 1%n" + @@ -281,6 +292,9 @@ public void testSubcommandUsesParentBundle() { " shared param1 desc line 1%n" + "subcmd option list heading%n" + " -h, --help Shared help option description.%n" + + " --optionWithDescriptionFromParent%n" + + " This description should be visible in both parent and sub%n" + + " commands%n" + " -V, --version Special version for i18n-sub.%n" + " -x, --xxx= X option description for i18n-sub%n" + " -y, --yyy= subcmd yyy description 1%n" + @@ -308,6 +322,47 @@ public void testSubcommandUsesParentBundle() { } } + @Test + public void testSubcommandAndParentCommandResourceBundlePrioritization() { + String expected = String.format("" + + "my name is sub2%n" + + "Shared header first line%n" + + "Shared header second line%n" + + "Usage: i18n-top sub2 [-hV] [--optionWithDescriptionFromParent] [-a=]%n" + + " [-q=] [-x=] [COMMAND]%n" + + "my parent is i18n-top, Shared description 0%n" + + "Shared description 1%n" + + "Shared description 2%n" + + " -a, --aaa= lorem ipsum%n" + + " -h, --help Shared help option description.%n" + + " --optionWithDescriptionFromParent%n" + + " This description should be visible in both parent and sub%n" + + " commands%n" + + " -q= This is the correct description for q%n" + + " -V, --version Print version information and exit.%n" + + " -x= shared X option description%n" + + "Commands:%n" + + " help Shared header first line%n" + + "Shared Exit Codes Heading%n" + + "These exit codes are blah blah etc.%n" + + " 00 (From shared bundle) Normal termination%n" + + " 64 (From shared bundle)%n" + + " Multiline!%n" + + " Invalid input%n" + + " 70 (From shared bundle) Internal error%n" + + "Shared footer heading%n" + + "lorem ipsum%n"); + Locale original = Locale.getDefault(); + Locale.setDefault(Locale.ENGLISH); + try { + CommandLine top = new CommandLine(new I18nCommand()); + CommandLine sub = top.getSubcommands().get("sub2"); + assertEquals(expected, sub.getUsageMessage()); + } finally { + Locale.setDefault(original); + } + } + @Test public void testGetResourceBundle_nullIfNotSpecified() { @Command class Noop {} @@ -590,7 +645,7 @@ public void testLocalizeBuiltInHelp_Shared() { "Shared header heading%n" + "i18n-sub HELP command header%n" + "%n" + - "Usage: i18n-top i18n-sub help [-h] [COMMAND]%n" + + "Usage: i18n-top i18n-sub help [-h] [--optionWithDescriptionFromParent] [COMMAND]%n" + "Shared description 0%n" + "Shared description 1%n" + "Shared description 2%n" + @@ -598,6 +653,9 @@ public void testLocalizeBuiltInHelp_Shared() { " subcommand%n" + " -h, --help Shared description of --help option of built-in help%n" + " subcommand%n" + + " --optionWithDescriptionFromParent%n" + + " This description should be visible in both parent and sub%n" + + " commands%n" + "Shared Exit Codes Heading%n" + "These exit codes are blah blah etc.%n" + " 00 (From shared bundle) Normal termination%n" + @@ -625,7 +683,7 @@ public void testLocalizeBuiltInHelp_Specialized() { "Shared header heading%n" + "i18n-top HELP command header%n" + "%n" + - "Usage: i18n-top help [-h] [COMMAND]%n" + + "Usage: i18n-top help [-h] [--optionWithDescriptionFromParent] [COMMAND]%n" + "Shared description 0%n" + "Shared description 1%n" + "Shared description 2%n" + @@ -633,6 +691,9 @@ public void testLocalizeBuiltInHelp_Specialized() { " subcommand%n" + " -h, --help Specialized description of --help option of i18-top help%n" + " subcommand%n" + + " --optionWithDescriptionFromParent%n" + + " This description should be visible in both parent and sub%n" + + " commands%n" + "Shared Exit Codes Heading%n" + "These exit codes are blah blah etc.%n" + " 00 (From shared bundle) Normal termination%n" + @@ -724,6 +785,13 @@ public void testTracingWithResourceBundle() { "[picocli DEBUG] Adding subcommand 'i18n-sub' to 'i18n-top'%n" + "[picocli DEBUG] Created Messages from resourceBundle[base=picocli.SharedMessages] for command 'i18n-sub' (command 'i18n-sub' (user object: class picocli.I18nSubcommand))%n" + "[picocli DEBUG] Created Messages from resourceBundle[base=picocli.SharedMessages] for command 'help' (command 'help' (user object: class picocli.CommandLine$HelpCommand))%n" + + "[picocli DEBUG] Creating CommandSpec for class picocli.I18nSubcommand2 with factory picocli.CommandLine$DefaultFactory%n" + + "[picocli DEBUG] Created Messages from resourceBundle[base=picocli.ResourceBundlePropagationTest] for command 'sub2' (command 'sub2' (user object: class picocli.I18nSubcommand2))%n" + + "[picocli DEBUG] Creating CommandSpec for class picocli.CommandLine$HelpCommand with factory picocli.CommandLine$DefaultFactory%n" + + "[picocli DEBUG] Adding subcommand 'help' to 'sub2'%n" + + "[picocli DEBUG] Created Messages from resourceBundle[base=picocli.ResourceBundlePropagationTest] for command 'help' (command 'help' (user object: class picocli.CommandLine$HelpCommand))%n" + + "[picocli DEBUG] Creating CommandSpec for picocli.CommandLine$AutoHelpMixin@ with factory picocli.CommandLine$DefaultFactory%n" + + "[picocli DEBUG] Adding subcommand 'sub2' to 'i18n-top'%n" + "[picocli DEBUG] Creating CommandSpec for picocli.CommandLine$AutoHelpMixin@34c53688 with factory picocli.CommandLine$DefaultFactory%n" + ""); assertEquals(stripAnsiTrace(stripHashcodes(expected)), stripAnsiTrace(stripHashcodes(err.toString()))); diff --git a/src/test/resources/picocli/ResourceBundlePropagationTest.properties b/src/test/resources/picocli/ResourceBundlePropagationTest.properties index d86f87254..71a7ad3b7 100644 --- a/src/test/resources/picocli/ResourceBundlePropagationTest.properties +++ b/src/test/resources/picocli/ResourceBundlePropagationTest.properties @@ -1,4 +1,5 @@ usage.headerHeading = my name is ${COMMAND-NAME}%n usage.descriptionHeading = my parent is ${PARENT-COMMAND-NAME},\u0020 usage.footer = ${additional.value} -additional.value = lorem ipsum \ No newline at end of file +additional.value = lorem ipsum +q = This is the correct description for q diff --git a/src/test/resources/picocli/SharedMessages.properties b/src/test/resources/picocli/SharedMessages.properties index 4963b8c53..28b32f166 100644 --- a/src/test/resources/picocli/SharedMessages.properties +++ b/src/test/resources/picocli/SharedMessages.properties @@ -41,3 +41,6 @@ i18n-top.help.helpCommand.command = Specialized description of COMMAND parameter helpCommand.help = Shared description of --help option of built-in help subcommand helpCommand.command = Shared description of COMMAND parameter of built-in help subcommand + +optionWithDescriptionFromParent = This description should be visible in both parent and sub commands +q = You should not be able to see this description for q in a subcommand diff --git a/src/test/resources/picocli/SharedMessages_ja.properties b/src/test/resources/picocli/SharedMessages_ja.properties index c20fc7046..d7067779e 100644 --- a/src/test/resources/picocli/SharedMessages_ja.properties +++ b/src/test/resources/picocli/SharedMessages_ja.properties @@ -42,3 +42,4 @@ i18n-top.help.helpCommand.command = i18-top \u30b3\u30de\u30f3\u30c9\u306eHELP\u helpCommand.help=Picocli\u306e\u5185\u90e8HELP\u30b5\u30d6\u30b3\u30de\u30f3\u30c9\u306e--help\u30aa\u30d7\u30b7\u30e7\u30f3\u306e\u5171\u901a\u8aac\u660e helpCommand.command=Picocli\u306e\u5185\u90e8HELP\u30b5\u30d6\u30b3\u30de\u30f3\u30c9\u306eCOMMAND\u30d1\u30e9\u30e1\u30fc\u30bf\u306e\u5171\u901a\u8aac\u660e +optionWithDescriptionFromParent = \u3053\u306e\u8aac\u660e\u306f\u3001\u89aa\u30b3\u30de\u30f3\u30c9\u3068\u30b5\u30d6\u30b3\u30de\u30f3\u30c9\u306e\u4e21\u65b9\u306b\u8868\u793a\u3055\u308c\u308b\u5fc5\u8981\u304c\u3042\u308a\u307e\u3059