Revxrsal / Lamp

A modern annotations-driven commands framework for Java and Kotlin
MIT License
202 stars 38 forks source link

[Bug] Default annotation conflicts with subcommand #13

Closed TehNeon closed 2 years ago

TehNeon commented 2 years ago

When the Default and Subcommand annotation are used together on the some method, it generates some weird conflicts. 1) The Default annotations routing functionality of the root/parent command is removed 2) The Default annotations "hidden" nature is still applied and hides the Subcommands aliases from the autocompleter

Properly functioning example:

@Command("default")
class DefaultCommand
{

    @Default
    fun default(sender: CommandSender, @Default("1") page: Int)
    {
        sender.sendMessage("Default subcommand, page: $page.")
    }

    @Subcommand("test")
    fun test(sender: CommandSender)
    {
        sender.sendMessage("Test subcommand!")
    }

}

When issuing commands: /default -> DefaultCommand#default /default test -> DefaultCommand#test

Conflicting version:

@Command("default")
class DefaultCommand
{

    @Default
    @Subcommand("help")
    fun default(sender: CommandSender, @Default("1") page: Int)
    {
        sender.sendMessage("Default subcommand, page: $page.")
    }
    }

    @Subcommand("test")
    fun test(sender: CommandSender)
    {
        sender.sendMessage("Test subcommand!")
    }

}

When issuing commands: /default -> Errors, subcommand must be specified /default help -> DefaultCommand#default /default test -> DefaultCommand#test

What I would expect it to do: /default -> DefaultCommand#default /default help -> DefaultCommand#default /default test -> DefaultCommand#test

With this conflicting version, the autocompleter also doesn't show that the help subcommand exists either and only provides the user with the test subcommand.

Revxrsal commented 2 years ago

Unfortunately this is the expected behavior for @Default. It creates a category at the given path and becomes the default action for it.

See this example

    @Default
    @Command("help")
    fun CommandActor.defaultHelp() {
        reply("Help")
    }

    @Command("help please")
    fun CommandActor.helpPlease() {
        reply("Help please")
    }

    @Command("help sir")
    fun CommandActor.helpSir() {
        reply("Help me sir")
    }

/help -> Help /help sudardu -> Help /help please -> Help please /help sir -> Help me sir

By your use case, it would be impossible for Lamp to tell apart whether it's the default command for its parent or the default command for the path it was given.

The way to fix this is to create a @Default method with the path of the parent. In your example, this would be:

@Command("default")
class DefaultCommand {

    @Default // appears in '/default'
    fun CommandActor.defaultHelp(@Default("1") page: Int) = help(page)

    @Subcommand("help")
    fun CommandActor.help(@Default("1") page: Int) {
        reply("Default help")
    }

    @Subcommand("test")
    fun CommandActor.test() {
        reply("Test subcommand!")
    }
}

In my opinion, attempting to guess which command to be the default for could lead to a lot of unexpected surprises and even bugs, therefore I think it's better to write a little more code in exchange for much more clarity and no surprises.