decline icon indicating copy to clipboard operation
decline copied to clipboard

Revisit Opts.subcommands

Open ricardogaspar2 opened this issue 4 years ago • 8 comments

Would you be willing to expose the SubCommand type the same way as Command ? It would be very useful to use it instead of its parent type Opts.

Currently, it's possible to create a command with Opts (arguments, or subcommands), it would be nice to be able to use the same approach for subcommands.

ricardogaspar2 avatar Aug 19 '20 16:08 ricardogaspar2

Hmm... could you say a bit more about your use case? Subcommand is basically just a wrapper around Command, so anything that's possible with a Command should work for subcommands "for free."

bkirwi avatar Aug 20 '20 00:08 bkirwi

Sure thing!

I've a big project for which I'm building a CLI with lots of commands and subcommands, similar to aws cli. And in order to be generic, extensible and help other teammates write and plug other commands/subcommands I created a few abstractions/types for it. For that reason, I'd like to enforce the definition of all implementations to be a subcommand, as the Opts is too generic.

These are the types I created:

This is a basic definition for each command I want to implement.

trait BaseJobCmdArgs {
  val commandName: String
  val helpMessage: String

  def command: Command[Job]
  def asSubcommand: Opts[Job] = { Opts.subcommand(command) }
}

As you can see, in order to guarantee that every implementation of this is a subcommand, I created the command function and an extra function asSubcommand just to force it to be a subcommand.

In order to have sub-sub commands (multiplexer/selector commands + it's subcommands) I had to go even further:


/**
 * Represents command that serves as a multiplexer to choose other jobs - represented as sub-commands.
 */
trait JobSelectorCmdArgs extends BaseJobCmdArgs {

  /**
   * List of subcommands for each job that this command will be able to redirect.
   * Each
   * @return
   */
  def jobSubCommands: Seq[BaseJobCmdArgs]

  override def command: Command[Job] = {
    // creating a command with subcommands
    Command(name = commandName, header = helpMessage, helpFlag = true)(
      jobSubCommands.map(_.asSubcommand).reduce(_.orElse(_))
    )
  }
}

/**
 * Represents a command for a job.
 */
trait JobCmdArgs extends BaseJobCmdArgs {
  val sparkAppName: String
}

And in the main this is how I invoke it:

import com.monovore.decline.{Command, Opts}
import com.typesafe.scalalogging.LazyLogging
import mystuff.Job

object MainCmdArgs extends LazyLogging {
 val name: String="mycli"
 val header: String="this is my cli"
 val subCommands: Seq[BaseJobCmdArgs] = Seq(JobCmd1,JobCmd2)

 def opts: Opts[Job] = subCommands.map(_.asSubcommand).reduce(_.orElse(_))

 def parse(args: Array[String]): Either[String, ob] =
   Command(name, header)(opts).parse(args, sys.env) match {
     case Left(help) =>
       Left(s"There's some error with the command you typed: '${args.mkString(" ")}'.\nError: $help")
     case Right(moduleJob: Job) => Right(moduleJob)
   }

 def job(args: Array[String]): Job = parse(args) match {
   case Left(help) =>
     logger.error(help)
     sys.exit(1)

   case Right(moduleJob) =>
     logger.debug(s"Successfully created job: $moduleJob")
     moduleJob
 }

 def main(args: Array[String]): Unit = {
   job(args).getJob.apply()
 }

}

This structure allows me to create a command hierarchy like this:

[ERROR] [MainCmdArgs.scala:23] There's some error with the command you typed: ''.
Error: Missing expected command (jobcmd1 or jobcmd2)!

Usage:
    mycli jobcmd1
    mycli jobcmd2

this is my cli

Options and flags:
    --help
        Display this help text.

Subcommands:
    jobcmd1
        To select other jobs.
    jobcmd1
        run Job2.

And for example, jobcmd1 can have it's own subcommands! If declared like this:

import JobCmdArgs, JobSelectorCmdArgs

object JobCmd1 extends JobSelectorCmdArgs {
  override val commandName: String = "jobcmd1"
  override val helpMessage: String = "To select other jobs."

  override def jobSubCommands: Seq[JobCmdArgs] =
    Seq(JobCmd3, JobCmd4)
}

And thus, instead of just having arguments for that jobcmd1, it will have subcommands which will have their own arguments:

[ERROR] [MainCmdArgs.scala:23] There's some error with the command you typed: 'jobcmd1'.
Error: Missing expected command (jobcmd3 or jobcmd4)!

Usage:
    mycli jobcmd1 jobcmd3
    mycli jobcmd1 jobcmd4


this is my cli

Options and flags:
    --help
        Display this help text.

Subcommands:
    jobcmd3
        run job 3.
    jobcmd4
        run job 4.

ricardogaspar2 avatar Aug 20 '20 12:08 ricardogaspar2

Thanks for the writeup! Always cool to see folks taking advantage of the composability of the library.

For that reason, I'd like to enforce the definition of all implementations to be a subcommand, as the Opts is too generic.

Makes sense! In this case, where you want every implementation to be a subcommand, I'd encourage you to use the Command type directly instead of any Opts subtype. Command and Subcommand are totally isomorphic, but a Command has some extra methods etc. that make it somewhat more useful. (And it's always trivial to convert to an Opts when you need to.)

For example, here's your code converted to use Command directly:

import com.monovore.decline.{Command, Opts}
import com.typesafe.scalalogging.LazyLogging
import mystuff.Job

object JobCmd1 {
  override def command: Command[Job] = Command("jobcmd1", "To select other jobs.") {
    Opts.subcommands(JobCmd3.command, JobCmd4.command)
  }
}

object MainCmdArgs extends LazyLogging {
 val name: String="mycli"
 val header: String="this is my cli"
 val subCommands: Seq[Command[Job]] = Seq(JobCmd1.command, JobCmd2.command)

 def opts: Opts[Job] = subCommands.map(Opts.subcommand).reduce(_.orElse(_))

 def parse(args: Array[String]): Either[String, Job] =
   Command(name, header)(opts).parse(args, sys.env) match {
     case Left(help) =>
       Left(s"There's some error with the command you typed: '${args.mkString(" ")}'.\nError: $help")
     case Right(moduleJob: Job) => Right(moduleJob)
   }

 def job(args: Array[String]): Job = parse(args) match {
   case Left(help) =>
     logger.error(help)
     sys.exit(1)

   case Right(moduleJob) =>
     logger.debug(s"Successfully created job: $moduleJob")
     moduleJob
 }

 def main(args: Array[String]): Unit = {
   job(args).getJob.apply()
 }
}

Should have the same behaviour!

bkirwi avatar Aug 20 '20 23:08 bkirwi

Hey, thanks for the quick reply!

While I waited for an answer, I was playing around and change my approach a bit - and coincidently applied some of your suggestions.

I used Opts.subcommands over the .map(Opts.subcommand).reduce(_.orElse(_)). From my experiments and tests it has the same meaning. I checked the source code and it appears so, is that right?

I had to use head and tail to make it work though: Opts.subcommands(subCommands.head.command, subCommands.tail.map(_.command): _*). And in order to prevent mistakes, I converted my subCommands Seq to a NonEmptyList.

How do you feel about having the another Opts.subcommands method accept a NonEmptyList?

ricardogaspar2 avatar Aug 21 '20 09:08 ricardogaspar2

I checked the source code and it appears so, is that right?

Yep, that's right! You could also use the cats syntax and do .map(Opts.subcommand).combineAll if you prefer.

How do you feel about having the another Opts.subcommands method accept a NonEmptyList?

Maybe! We could change the existing method to take an A* -- you wouldn't expect folks to use the empty-list case much in practise, but it simplifies things a little bit.

I don't like too much sugar for stuff like this... the implementation you have is just slightly longer than the helper method, and it's a little bit more "revealing." (ie. it shows that having multiple subcommands is just a special case of a more general feature.). But it is nice to keep simple cases simple. I'll think about it!

bkirwi avatar Aug 21 '20 22:08 bkirwi

Thank you for all the answers, feedback and openness! No need for this issue to be open anymore then :)

Keep up with the great work! 👌

ricardogaspar2 avatar Aug 24 '20 10:08 ricardogaspar2

Thanks! Left this open to remind me to revisit that subcommands signature when we do a major release.

bkirwi avatar Aug 28 '20 02:08 bkirwi

hey again @bkirwi 👋 Since you released a new major, does it make sense for you to revisit this ☝️ ?

ricardogaspar2 avatar Sep 01 '21 15:09 ricardogaspar2