stevedlawrence commented on code in PR #800:
URL: https://github.com/apache/daffodil/pull/800#discussion_r891133863
##########
daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala:
##########
@@ -397,6 +407,10 @@ class CLIConf(arguments: Array[String]) extends
scallop.ScallopConf(arguments) {
descr("List or execute TDML tests")
helpWidth(width)
+ val implementation = opt[TDMLImplementation](short = 'I', argName =
"implementation",
+ descr = "Implementation to run TDML tests. Choose one of %s. Defaults to
%s."
+ .format(TDMLImplementation.allValues.mkString(", "),
TDMLImplementation.allValues.head.toString),
+ default = TDMLImplementation.optionStringToEnum("I",
sys.env.getOrElse("TDML_IMPLEMENTATION", "")))
Review Comment:
I see, so the default could potentially be None if TDML_IMPLEMENTATION is
invalid, in which case it's ignored and "daffodil" is used. I find that a
little intuitive with the empty string, as well as ignoring invalid
TDML_IMPLEMENTATIONS. Thoughts on this alternative approach below?
First, we set `default = TDMLImplementation.Daffodil` in the scallop config.
Then the code that uses this below looks something like
```scala
val tdmlImlementation =
if (testOpts.implementation.isSupplied) testOpts.implementation() // use
-I if provided
else {
val env = sys.env.get("TDML_IMPLEMENTATION").flatMap { s =>
val optEnum =
TDMLImplementation.optionStringToEnum("--implementation", s)
if (!optEnum.isDefined) { ... } // invalid, warn or error
optEnum
}
env.getOrElse(testOpts.implementation()) // get the default if env is
invalid or not defined
}
```
It's definitely more verbose, but maybe it's more clear what the logic is?
I'm not sure.
The `-I ibm` behavior feels wrong to me, seem like it could be very
confusing to specify `-I ibm` and we actually run with Daffodil. But I guess
there's not much we can do about it since the IBM and Daffodil implementations
use the same class name. Maybe that should change so they don't have the same
name? But that is probably something to fix separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
