#293 Translatable help - show translated description of child commands

- Show translated command descriptions when available
- Fix bug where localized command is registered on the parent each time

Thanks to @Maxetto

src/main/java/fr/xephi/authme/command/CommandDescription.java

@@ -56,16 +56,10 @@ public class CommandDescription {
     private PermissionNode permission;
 
     /**
-     * Private constructor. Use {@link CommandDescription#builder()} to create instances of this class.
+     * Private constructor.
      * <p>
-     * Note for developers: Instances should be created with {@link CommandDescription#createInstance} to be properly
+     * Note for developers: Instances should be created with {@link CommandBuilder#register()} to be properly
      * registered in the command tree.
-     */
-    private CommandDescription() {
-    }
-
-    /**
-     * Create an instance.
      *
      * @param labels              command labels
      * @param description         description of the command
@@ -74,30 +68,17 @@ public class CommandDescription {
      * @param parent              parent command
      * @param arguments           command arguments
      * @param permission          permission node required to execute this command
-     *
-     * @return the created instance
-     * @see CommandDescription#builder()
      */
-    private static CommandDescription createInstance(List<String> labels, String description,
+    private CommandDescription(List<String> labels, String description, String detailedDescription, 
-            String detailedDescription, Class<? extends ExecutableCommand> executableCommand, CommandDescription parent,
+                               Class<? extends ExecutableCommand> executableCommand, CommandDescription parent,
-            List<CommandArgumentDescription> arguments,  PermissionNode permission) {
+                               List<CommandArgumentDescription> arguments,  PermissionNode permission) {
-        CommandDescription instance = new CommandDescription();
+        this.labels = labels;
-        instance.labels = labels;
+        this.description = description;
-        instance.description = description;
+        this.detailedDescription = detailedDescription;
-        instance.detailedDescription = detailedDescription;
+        this.executableCommand = executableCommand;
-        instance.executableCommand = executableCommand;
+        this.parent = parent;
-        instance.parent = parent;
+        this.arguments = arguments;
-        instance.arguments = arguments;
+        this.permission = permission;
-        instance.permission = permission;
-
-        if (parent != null) {
-            parent.addChild(instance);
-        }
-        return instance;
-    }
-
-    private void addChild(CommandDescription command) {
-        children.add(command);
     }
 
     /**
@@ -224,8 +205,21 @@ public class CommandDescription {
         private PermissionNode permission;
 
         /**
-         * Build a CommandDescription from the builder or throw an exception if a mandatory
+         * Build a CommandDescription and register it onto the parent if available.
-         * field has not been set.
+         *
+         * @return The generated CommandDescription object
+         */
+        public CommandDescription register() {
+            CommandDescription command = build();
+
+            if (command.parent != null) {
+                command.parent.children.add(command);
+            }
+            return command;
+        }
+
+        /**
+         * Build a CommandDescription (without registering it on the parent).
          *
          * @return The generated CommandDescription object
          */
@@ -236,8 +230,8 @@ public class CommandDescription {
             checkArgument(executableCommand != null, "Executable command must be set");
             // parents and permissions may be null; arguments may be empty
 
-            return createInstance(labels, description, detailedDescription, executableCommand,
+            return new CommandDescription(labels, description, detailedDescription, executableCommand,
-                                  parent, arguments, permission);
+                                          parent, arguments, permission);
         }
 
         public CommandBuilder labels(List<String> labels) {

src/main/java/fr/xephi/authme/command/CommandInitializer.java

@@ -70,7 +70,7 @@ public class CommandInitializer {
             .description("Main command")
             .detailedDescription("The main AuthMeReloaded command. The root for all admin commands.")
             .executableCommand(AuthMeCommand.class)
-            .build();
+            .register();
 
         // Register the register command
         CommandDescription.builder()
@@ -82,7 +82,7 @@ public class CommandInitializer {
             .withArgument("password", "Password", false)
             .permission(AdminPermission.REGISTER)
             .executableCommand(RegisterAdminCommand.class)
-            .build();
+            .register();
 
         // Register the unregister command
         CommandDescription.builder()
@@ -93,7 +93,7 @@ public class CommandInitializer {
             .withArgument("player", "Player name", false)
             .permission(AdminPermission.UNREGISTER)
             .executableCommand(UnregisterAdminCommand.class)
-            .build();
+            .register();
 
         // Register the forcelogin command
         CommandDescription.builder()
@@ -104,7 +104,7 @@ public class CommandInitializer {
             .withArgument("player", "Online player name", true)
             .permission(AdminPermission.FORCE_LOGIN)
             .executableCommand(ForceLoginCommand.class)
-            .build();
+            .register();
 
         // Register the changepassword command
         CommandDescription.builder()
@@ -116,7 +116,7 @@ public class CommandInitializer {
             .withArgument("pwd", "New password", false)
             .permission(AdminPermission.CHANGE_PASSWORD)
             .executableCommand(ChangePasswordAdminCommand.class)
-            .build();
+            .register();
 
         // Register the last login command
         CommandDescription.builder()
@@ -127,7 +127,7 @@ public class CommandInitializer {
             .withArgument("player", "Player name", true)
             .permission(AdminPermission.LAST_LOGIN)
             .executableCommand(LastLoginCommand.class)
-            .build();
+            .register();
 
         // Register the accounts command
         CommandDescription.builder()
@@ -138,7 +138,7 @@ public class CommandInitializer {
             .withArgument("player", "Player name or IP", true)
             .permission(AdminPermission.ACCOUNTS)
             .executableCommand(AccountsCommand.class)
-            .build();
+            .register();
 
         // Register the getemail command
         CommandDescription.builder()
@@ -149,7 +149,7 @@ public class CommandInitializer {
             .withArgument("player", "Player name", true)
             .permission(AdminPermission.GET_EMAIL)
             .executableCommand(GetEmailCommand.class)
-            .build();
+            .register();
 
         // Register the setemail command
         CommandDescription.builder()
@@ -161,7 +161,7 @@ public class CommandInitializer {
             .withArgument("email", "Player email", false)
             .permission(AdminPermission.CHANGE_EMAIL)
             .executableCommand(SetEmailCommand.class)
-            .build();
+            .register();
 
         // Register the getip command
         CommandDescription.builder()
@@ -172,7 +172,7 @@ public class CommandInitializer {
             .withArgument("player", "Player name", false)
             .permission(AdminPermission.GET_IP)
             .executableCommand(GetIpCommand.class)
-            .build();
+            .register();
 
         // Register the spawn command
         CommandDescription.builder()
@@ -182,7 +182,7 @@ public class CommandInitializer {
             .detailedDescription("Teleport to the spawn.")
             .permission(AdminPermission.SPAWN)
             .executableCommand(SpawnCommand.class)
-            .build();
+            .register();
 
         // Register the setspawn command
         CommandDescription.builder()
@@ -192,7 +192,7 @@ public class CommandInitializer {
             .detailedDescription("Change the player's spawn to your current position.")
             .permission(AdminPermission.SET_SPAWN)
             .executableCommand(SetSpawnCommand.class)
-            .build();
+            .register();
 
         // Register the firstspawn command
         CommandDescription.builder()
@@ -202,7 +202,7 @@ public class CommandInitializer {
             .detailedDescription("Teleport to the first spawn.")
             .permission(AdminPermission.FIRST_SPAWN)
             .executableCommand(FirstSpawnCommand.class)
-            .build();
+            .register();
 
         // Register the setfirstspawn command
         CommandDescription.builder()
@@ -212,7 +212,7 @@ public class CommandInitializer {
             .detailedDescription("Change the first player's spawn to your current position.")
             .permission(AdminPermission.SET_FIRST_SPAWN)
             .executableCommand(SetFirstSpawnCommand.class)
-            .build();
+            .register();
 
         // Register the purge command
         CommandDescription.builder()
@@ -224,7 +224,7 @@ public class CommandInitializer {
             .withArgument("all", "Add 'all' at the end to also purge players with lastlogin = 0", true)
             .permission(AdminPermission.PURGE)
             .executableCommand(PurgeCommand.class)
-            .build();
+            .register();
 
         // Register the purgelastposition command
         CommandDescription.builder()
@@ -236,7 +236,7 @@ public class CommandInitializer {
             .withArgument("player/*", "Player name or * for all players", false)
             .permission(AdminPermission.PURGE_LAST_POSITION)
             .executableCommand(PurgeLastPositionCommand.class)
-            .build();
+            .register();
 
         // Register the purgebannedplayers command
         CommandDescription.builder()
@@ -246,7 +246,7 @@ public class CommandInitializer {
             .detailedDescription("Purge all AuthMeReloaded data for banned players.")
             .permission(AdminPermission.PURGE_BANNED_PLAYERS)
             .executableCommand(PurgeBannedPlayersCommand.class)
-            .build();
+            .register();
 
         // Register the switchantibot command
         CommandDescription.builder()
@@ -257,7 +257,7 @@ public class CommandInitializer {
             .withArgument("mode", "ON / OFF", true)
             .permission(AdminPermission.SWITCH_ANTIBOT)
             .executableCommand(SwitchAntiBotCommand.class)
-            .build();
+            .register();
 
         // Register the reload command
         CommandDescription.builder()
@@ -267,7 +267,7 @@ public class CommandInitializer {
             .detailedDescription("Reload the AuthMeReloaded plugin.")
             .permission(AdminPermission.RELOAD)
             .executableCommand(ReloadCommand.class)
-            .build();
+            .register();
 
         // Register the version command
         CommandDescription.builder()
@@ -277,7 +277,7 @@ public class CommandInitializer {
             .detailedDescription("Show detailed information about the installed AuthMeReloaded version, the "
                 + "developers, contributors, and license.")
             .executableCommand(VersionCommand.class)
-            .build();
+            .register();
 
         CommandDescription.builder()
             .parent(AUTHME_BASE)
@@ -288,7 +288,7 @@ public class CommandInitializer {
                 "royalauth / vauth / sqliteToSql / mysqlToSqlite", false)
             .permission(AdminPermission.CONVERTER)
             .executableCommand(ConverterCommand.class)
-            .build();
+            .register();
 
         CommandDescription.builder()
             .parent(AUTHME_BASE)
@@ -297,7 +297,7 @@ public class CommandInitializer {
             .detailedDescription("Adds missing messages to the current messages file.")
             .permission(AdminPermission.UPDATE_MESSAGES)
             .executableCommand(MessagesCommand.class)
-            .build();
+            .register();
 
         // Register the base login command
         final CommandDescription LOGIN_BASE = CommandDescription.builder()
@@ -308,7 +308,7 @@ public class CommandInitializer {
             .withArgument("password", "Login password", false)
             .permission(PlayerPermission.LOGIN)
             .executableCommand(LoginCommand.class)
-            .build();
+            .register();
 
         // Register the base logout command
         CommandDescription LOGOUT_BASE = CommandDescription.builder()
@@ -318,7 +318,7 @@ public class CommandInitializer {
             .detailedDescription("Command to logout using AuthMeReloaded.")
             .permission(PlayerPermission.LOGOUT)
             .executableCommand(LogoutCommand.class)
-            .build();
+            .register();
 
         // Register the base register command
         final CommandDescription REGISTER_BASE = CommandDescription.builder()
@@ -330,7 +330,7 @@ public class CommandInitializer {
             .withArgument("verifyPassword", "Verify password", true)
             .permission(PlayerPermission.REGISTER)
             .executableCommand(RegisterCommand.class)
-            .build();
+            .register();
 
         // Register the base unregister command
         CommandDescription UNREGISTER_BASE = CommandDescription.builder()
@@ -341,7 +341,7 @@ public class CommandInitializer {
             .withArgument("password", "Password", false)
             .permission(PlayerPermission.UNREGISTER)
             .executableCommand(UnregisterCommand.class)
-            .build();
+            .register();
 
         // Register the base changepassword command
         final CommandDescription CHANGE_PASSWORD_BASE = CommandDescription.builder()
@@ -353,7 +353,7 @@ public class CommandInitializer {
             .withArgument("newPassword", "New Password.", false)
             .permission(PlayerPermission.CHANGE_PASSWORD)
             .executableCommand(ChangePasswordCommand.class)
-            .build();
+            .register();
 
         // Register the base Email command
         CommandDescription EMAIL_BASE = CommandDescription.builder()
@@ -362,7 +362,7 @@ public class CommandInitializer {
             .description("Email command")
             .detailedDescription("The AuthMeReloaded Email command base.")
             .executableCommand(EmailBaseCommand.class)
-            .build();
+            .register();
 
         // Register the show command
         CommandDescription.builder()
@@ -371,7 +371,7 @@ public class CommandInitializer {
             .description("Show Email")
             .detailedDescription("Show your current email address.")
             .executableCommand(ShowEmailCommand.class)
-            .build();
+            .register();
 
         // Register the add command
         CommandDescription.builder()
@@ -383,7 +383,7 @@ public class CommandInitializer {
             .withArgument("verifyEmail", "Email address verification", false)
             .permission(PlayerPermission.ADD_EMAIL)
             .executableCommand(AddEmailCommand.class)
-            .build();
+            .register();
 
         // Register the change command
         CommandDescription.builder()
@@ -395,7 +395,7 @@ public class CommandInitializer {
             .withArgument("newEmail", "New email address", false)
             .permission(PlayerPermission.CHANGE_EMAIL)
             .executableCommand(ChangeEmailCommand.class)
-            .build();
+            .register();
 
         // Register the recover command
         CommandDescription.builder()
@@ -408,7 +408,7 @@ public class CommandInitializer {
             .withArgument("code", "Recovery code", true)
             .permission(PlayerPermission.RECOVER_EMAIL)
             .executableCommand(RecoverEmailCommand.class)
-            .build();
+            .register();
 
         // Register the base captcha command
         CommandDescription CAPTCHA_BASE = CommandDescription.builder()
@@ -419,7 +419,7 @@ public class CommandInitializer {
             .withArgument("captcha", "The Captcha", false)
             .permission(PlayerPermission.CAPTCHA)
             .executableCommand(CaptchaCommand.class)
-            .build();
+            .register();
 
         Set<CommandDescription> baseCommands = ImmutableSet.of(
             AUTHME_BASE,
@@ -451,7 +451,7 @@ public class CommandInitializer {
                 .detailedDescription("View detailed help for /" + base.getLabels().get(0) + " commands.")
                 .withArgument("query", "The command or query to view help for.", true)
                 .executableCommand(HelpCommand.class)
-                .build();
+                .register();
         }
     }
 }

src/main/java/fr/xephi/authme/command/help/HelpMessagesService.java

@@ -67,6 +67,10 @@ public class HelpMessagesService implements Reloadable {
         return localCommand;
     }
 
+    public String getDescription(CommandDescription command) {
+        return getText(getCommandPath(command) + DESCRIPTION_SUFFIX, command::getDescription);
+    }
+
     public String getMessage(HelpMessage message) {
         return messageFileHandler.getMessage(message.getKey());
     }

src/main/java/fr/xephi/authme/command/help/HelpProvider.java

@@ -249,7 +249,7 @@ public class HelpProvider implements Reloadable {
         String parentCommandPath = String.join(" ", parentLabels);
         for (CommandDescription child : command.getChildren()) {
             lines.add(" /" + parentCommandPath + " " + child.getLabels().get(0)
-                + ChatColor.GRAY + ChatColor.ITALIC + ": " + child.getDescription());
+                + ChatColor.GRAY + ChatColor.ITALIC + ": " + helpMessagesService.getDescription(child));
         }
     }
 

src/main/java/fr/xephi/authme/permission/PlayerPermission.java

@@ -41,7 +41,7 @@ public enum PlayerPermission implements PermissionNode {
     CHANGE_EMAIL("authme.player.email.change"),
 
     /**
-     * Command permission to recover an account using it's email address.
+     * Command permission to recover an account using its email address.
      */
     RECOVER_EMAIL("authme.player.email.recover"),
 

src/test/java/fr/xephi/authme/command/CommandUtilsTest.java

@@ -19,14 +19,14 @@ public class CommandUtilsTest {
             .description("Base")
             .detailedDescription("Test base command.")
             .executableCommand(ExecutableCommand.class)
-            .build();
+            .register();
         CommandDescription command = CommandDescription.builder()
             .parent(base)
             .labels("help", "h", "?")
             .description("Child")
             .detailedDescription("Test child command.")
             .executableCommand(ExecutableCommand.class)
-            .build();
+            .register();
 
         // when
         String commandPath = CommandUtils.constructCommandPath(command);
@@ -42,7 +42,7 @@ public class CommandUtilsTest {
     @Test
     public void shouldComputeMinAndMaxOnEmptyCommand() {
         // given
-        CommandDescription command = getBuilderForArgsTest().build();
+        CommandDescription command = getBuilderForArgsTest().register();
 
         // when / then
         checkArgumentCount(command, 0, 0);
@@ -54,7 +54,7 @@ public class CommandUtilsTest {
         CommandDescription command = getBuilderForArgsTest()
             .withArgument("Test", "Arg description", false)
             .withArgument("Test22", "Arg description 2", false)
-            .build();
+            .register();
 
         // when / then
         checkArgumentCount(command, 2, 2);
@@ -67,7 +67,7 @@ public class CommandUtilsTest {
             .withArgument("arg1", "Arg description", false)
             .withArgument("arg2", "Arg description 2", true)
             .withArgument("arg3", "Arg description 3", true)
-            .build();
+            .register();
 
         // when / then
         checkArgumentCount(command, 1, 3);

src/test/java/fr/xephi/authme/command/TestCommandsUtil.java

@@ -42,7 +42,7 @@ public final class TestCommandsUtil {
             newArgument("player", true));
         // Register /email helptest -- use only to test for help command arguments special case
         CommandDescription.builder().parent(emailBase).labels("helptest").executableCommand(HelpCommand.class)
-            .description("test").detailedDescription("Test.").withArgument("Query", "", false).build();
+            .description("test").detailedDescription("Test.").withArgument("Query", "", false).register();
 
         // Register /unregister <player>, alias: /unreg
         CommandDescription unregisterBase = createCommand(AdminPermission.UNREGISTER, null,
@@ -101,7 +101,7 @@ public final class TestCommandsUtil {
             }
         }
 
-        return command.build();
+        return command.register();
     }
 
     /** Shortcut command to initialize a new argument description. */

src/test/java/fr/xephi/authme/command/help/HelpMessagesServiceTest.java

@@ -16,6 +16,7 @@ import java.util.Set;
 import java.util.function.Function;
 
 import static fr.xephi.authme.TestHelper.getJarFile;
+import static fr.xephi.authme.command.TestCommandsUtil.getCommandWithLabel;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.sameInstance;
@@ -48,7 +49,7 @@ public class HelpMessagesServiceTest {
     @Test
     public void shouldReturnLocalizedCommand() {
         // given
-        CommandDescription command = TestCommandsUtil.getCommandWithLabel(COMMANDS, "authme", "register");
+        CommandDescription command = getCommandWithLabel(COMMANDS, "authme", "register");
 
         // when
         CommandDescription localCommand = helpMessagesService.buildLocalizedDescription(command);
@@ -68,7 +69,7 @@ public class HelpMessagesServiceTest {
     @Test
     public void shouldReturnLocalizedCommandWithDefaults() {
         // given
-        CommandDescription command = TestCommandsUtil.getCommandWithLabel(COMMANDS, "authme", "login");
+        CommandDescription command = getCommandWithLabel(COMMANDS, "authme", "login");
 
         // when
         CommandDescription localCommand = helpMessagesService.buildLocalizedDescription(command);
@@ -84,7 +85,7 @@ public class HelpMessagesServiceTest {
     @Test
     public void shouldReturnSameCommandForNoLocalization() {
         // given
-        CommandDescription command = TestCommandsUtil.getCommandWithLabel(COMMANDS, "email");
+        CommandDescription command = getCommandWithLabel(COMMANDS, "email");
 
         // when
         CommandDescription localCommand = helpMessagesService.buildLocalizedDescription(command);
@@ -96,7 +97,7 @@ public class HelpMessagesServiceTest {
     @Test
     public void shouldKeepChildrenInLocalCommand() {
         // given
-        CommandDescription command = TestCommandsUtil.getCommandWithLabel(COMMANDS, "authme");
+        CommandDescription command = getCommandWithLabel(COMMANDS, "authme");
 
         // when
         CommandDescription localCommand = helpMessagesService.buildLocalizedDescription(command);
@@ -114,4 +115,28 @@ public class HelpMessagesServiceTest {
         assertThat(helpMessagesService.getMessage(HelpMessage.RESULT), equalTo("res."));
         assertThat(helpMessagesService.getMessage(HelpSection.ARGUMENTS), equalTo("arg."));
     }
+
+    @Test
+    public void shouldGetLocalCommandDescription() {
+        // given
+        CommandDescription command = getCommandWithLabel(COMMANDS, "authme", "register");
+
+        // when
+        String description = helpMessagesService.getDescription(command);
+
+        // then
+        assertThat(description, equalTo("Registration"));
+    }
+
+    @Test
+    public void shouldFallbackToDescriptionOnCommandObject() {
+        // given
+        CommandDescription command = getCommandWithLabel(COMMANDS, "unregister");
+
+        // when
+        String description = helpMessagesService.getDescription(command);
+
+        // then
+        assertThat(description, equalTo(command.getDescription()));
+    }
 }

src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java

@@ -251,6 +251,10 @@ public class HelpProviderTest {
         // given
         CommandDescription command = getCommandWithLabel(commands, "authme");
         FoundCommandResult result = newFoundResult(command, Collections.singletonList("authme"));
+        given(helpMessagesService.getDescription(getCommandWithLabel(commands, "authme", "login")))
+            .willReturn("Command for login [localized]");
+        given(helpMessagesService.getDescription(getCommandWithLabel(commands, "authme", "register")))
+            .willReturn("Registration command [localized]");
 
         // when
         helpProvider.outputHelp(sender, result, SHOW_CHILDREN);
@@ -258,9 +262,9 @@ public class HelpProviderTest {
         // then
         List<String> lines = getLines(sender);
         assertThat(lines, hasSize(4));
-        assertThat(lines.get(1), containsString("Children:"));
+        assertThat(lines.get(1), equalTo("Children:"));
-        assertThat(lines.get(2), containsString("/authme login: login cmd"));
+        assertThat(lines.get(2), equalTo(" /authme login: Command for login [localized]"));
-        assertThat(lines.get(3), containsString("/authme register: register cmd"));
+        assertThat(lines.get(3), equalTo(" /authme register: Registration command [localized]"));
     }
 
     @Test