Tooling project #76
Tooling project #76
Conversation
|
I suggest we use something similar to https://github.com/scala-native/scala-native/blob/master/tools/src/main/scala/scala/scalanative/build/Config.scala i.e. a |
| import java.io.{File, PrintWriter} | ||
|
|
||
| class Bindings(private val bindings: String) { | ||
| def writeToFile(pathToFile: String): Unit = { |
There was a problem hiding this comment.
Using java.nio.file.Path here would be a better fit.
There was a problem hiding this comment.
I suppose just File is also fine
| /** | ||
| * Set header file for which bindings will be generated | ||
| */ | ||
| def header(pathToHeader: String): Unit = { |
There was a problem hiding this comment.
Similarly, let's use java.nio.file.Path here to indicate the type properly.
| * contain given prefix | ||
| */ | ||
| def excludePrefix(prefix: String): Unit = { | ||
| if (!prefix.isEmpty) { |
There was a problem hiding this comment.
If you think we need validation code, then let's use require to fail on invalid input.
| private var packageName: String = _ | ||
| private var excludePrefix: String = _ | ||
| private val extraArg: mutable.Seq[String] = mutable.Seq() | ||
| private val extraArgBefore: mutable.Seq[String] = mutable.Seq() |
There was a problem hiding this comment.
I'd much prefer to move these to the case class members:
private final case class Impl(
execuable: File,
library: String,
// ...
extraArgs: immutable.Seq[String],
// ...
) extends Bindgen {
def bindgenExecutable((executable: File): Bindgen {
require(executable.exists())
copy(executable = executable)
}
def extraArgs(args: String*): Bindgen = {
require(args.forall(_.nonEmpty))
copy(extraArgs = extraArgs ++ args)
}
// ...
}This also allows to use immutable.Seq or List.
| @@ -1,9 +1,8 @@ | |||
| package org.scalanative.bindgen | |||
|
|
|||
| import java.io.{File, PrintWriter} | |||
| import java.io.{File} | |||
| * Name of Scala object that contains bindings. | ||
| * Default is set to library name. | ||
| */ | ||
| def scalaObjectName(scalaObjectName: String): Bindgen |
| } | ||
|
|
||
| private def validateFields(): Unit = { | ||
| if (executable == null) { |
There was a problem hiding this comment.
Using require(executable.isDefined, "The executable must be specified") is more concise.
| executable: File = null, | ||
| library: String = null, | ||
| header: File = null, | ||
| scalaObjectName: String = null, |
There was a problem hiding this comment.
Optional fields are better expressed using Option[T] so the compiler forces them to be handled explicitly.
| def apply(): Bindgen = Impl() | ||
|
|
||
| private final case class Impl( | ||
| executable: File = null, |
There was a problem hiding this comment.
How about using Option[File] and default to scala-native-bindgen
executable.fold("scala-native-bindgen")(_.getAbsolutePath)There was a problem hiding this comment.
But then we should warn that default value is set because user might simply forget to set executable, so I would not use default
| if (this.scalaObjectName != null) | ||
| this.scalaObjectName | ||
| else | ||
| library |
There was a problem hiding this comment.
With Option[String] as the type this becomes
val name = this.name.getOrElse(library)| .packageName("org.scalanative.bindgen.samples") | ||
| .excludePrefix("__") | ||
| .generate() | ||
| .writeToFile(outputFile) |
Part of #60
Currently contains only API without proper implementation