Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commands parameter parser should support delegate classes #1877

Closed
lefou opened this issue May 16, 2022 · 3 comments
Closed

Commands parameter parser should support delegate classes #1877

lefou opened this issue May 16, 2022 · 3 comments

Comments

@lefou
Copy link
Member

lefou commented May 16, 2022

Because the way commands are currently implemented, there is no well defined way to overload a command with different options. This makes gradual evolving a modules API hard.

mainargs already supports parsing case classes. If Mill would support parsing of command parameter by delegating to parsing a case class first, we could gradually evolve and deprecate some parameters without breaking (binary) compatibility.

Example:

case class MyArgs(verbose: Flag = Flag, prefix: Option[String] = None)
def myCommand(args: MyArgs) = T.command {
  if(args.verbose.value) ...
}
$ mill myCommand --verbose --prefix abc
@lefou
Copy link
Member Author

lefou commented Jul 12, 2022

@lihaoyi Could you comment whether this is already possible or how much work this might take in either mainargs or Mill?

@lefou
Copy link
Member Author

lefou commented Sep 11, 2022

To really avoid binary compatibility issue, case classes are a bad choice. We rather want to use ordinary classes, so we probably need to add support for these in mainargs.

@lefou
Copy link
Member Author

lefou commented Sep 12, 2022

Haha, this is actually working, when not defined in a build.sc. It fails to compile in a build.sc, probably because the code wrapping happening for Ammonite scripts. But it works, if we define the module trait in a Scala class and compile it in a regular project.

trait MyModule extends Module {
  def argTest(args: MyModule.MyArgs) = T.command {
    println(s"args: ${args}")
  }
}

object MyModule {
  case class MyArgs(a: mainargs.Flag = mainargs.Flag(false), b: Seq[String] = Seq.empty[String])
  implicit val argsReader: mainargs.ParserForClass[MyArgs] = mainargs.ParserForClass[MyArgs]
}

It's even working for non-case classes, as long as we provide a companion object with an apply method.

trait MyModule extends Module {
  def argTest(args: MyModule.MyArgs) = T.command {
    println(s"args: ${args}")
  }
}

object MyModule {
  class MyArgs private (val a: mainargs.Flag, val b: Seq[String]) {
    override def toString: String = getClass.getSimpleName + ("a=" + a + ",b=" + b + ")")
  }

  object MyArgs {
    @deprecated("test deprecation")
    def apply(a: mainargs.Flag): MyArgs = apply(a, Seq.empty)

    def apply(a: mainargs.Flag = mainargs.Flag(false), b: Seq[String] = Seq.empty): MyArgs =
      new MyArgs(a, b)
  }

  implicit val argsReader: mainargs.ParserForClass[MyArgs] = mainargs.ParserForClass[MyArgs]
}

@lefou lefou closed this as completed Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant