Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Commit

Permalink
Various fixes for usages derived from the @arg annotation.
Browse files Browse the repository at this point in the history
Enforce if both 'flag' and 'name' are given in the `@arg` annotation, the
length of 'flag' is less than or equal to the length of 'name'.  It is ok
if the length of 'name' derived from the constructor-parameter/field is
less than the length of 'flag', and in this cause we'll print 'flag'
second. Examples:
1. `@arg(flag="abc", name="a") val a: T` should be illegal
2. `@arg(flag="abc") val a: T` will have usage "-a T, --abc=T"
Previously it would not check and print the following usages for 1-2
above:
1. "-abc T, --a=T". Note that specifying either "-abc T" or "--a T"
wouldn't work.
2. "-abc T, -a=T". Note that specifying "-abc T" wouldn't work.

Format a single-character flag correctly if specified in name or the
variable/field is a single-character.
- `@arg(name="a") val abc: T` will have usage "-a T".
- `@arg val a: T` will have usage "-a T".

Add a regression test for the 'name' field being camel case:
- `@arg(name="aB") val a: T` will have usage "--aB=T".

Fix the usage for multi-character flag arguments.  For example, if
`@arg(flag="abc") val abcd: T` then the usage will be
"--abc=T, --abcd=T"; it was previously "-abc T, --abcd=T".

Make it ok for the 'flag' and derived 'name' are the same, but only
print out one: `@arg(flag="a") val a: T` will be "-a T".

Updated the documentation for ArgAnnotation for all the above.
  • Loading branch information
nh13 committed Aug 17, 2016
1 parent 65153dd commit 81db51b
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 16 deletions.
13 changes: 9 additions & 4 deletions sopt/src/main/java/dagr/sopt/cmdline/ArgAnnotation.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,20 @@
* prefixed on the command-line with a double dash (--). If not specified
* then default is used, which is to translate the field name into a GNU style
* option name by breaking words on camel case, joining words back together with
* hyphens, and converting to lower case (e.g. myThing => my-thing).
* hyphens, and converting to lower case (e.g. myThing => my-thing). The
* resulting value should be different than {@link #flag()}.
* @return Selected full name, or "" to use the default.
*/
String name() default "";

/**
* Specified short name of the command. Short names should be prefixed
* with a single dash. ArgAnnotation values can directly abut single-char
* short names or be separated from them by a space.
* Specified short name of the command. Single value short names should be
* prefixed on the command-line with a single dash. Multi-character short-names
* will be treated like {@link #name()}. ArgAnnotation values can directly abut
* single-char short names or be separated from them by a space. The short name
* should always have length less than or equal to {@link #name()} if the latter
* is not empty and is not derived from the field name. Regardless, they should
* not have the same value.
* @return Selected short name, or "" for none.
*/
String flag() default "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,28 @@ object ClpArgumentDefinitionPrinting {
private val ArgumentColumnWidth: Int = 30
private val DescriptionColumnWidth: Int = 90

/** Formats a short or long name, depending on the length of the name. */
private def formatName(name: String, theType: String): String = {
if (name.length == 1) {
val nameType = if (theType == "Boolean") "[true|false]" else theType
"-" + name + " " + nameType
}
else {
val nameType = if (theType == "Boolean") "[=true|false]" else "=" + theType
"--" + name + nameType
}
}

/** Prints the usage for a given argument given its various elements */
private def printArgumentUsage(stringBuilder: StringBuilder, name: String, shortName: String, theType: String, argumentDescription: String): Unit = {
// Desired output: "-f Foo, --foo=Foo" and for Booleans, "-f [true|false] --foo=[true|false]"
val (shortType, longType) = if (theType == "Boolean") ("[true|false]","[=true|false]") else (theType, "=" + theType)
val label = new StringBuilder()
if (shortName.nonEmpty) label.append("-" + shortName + " " + shortType + ", ")
label.append("--" + name + longType)
stringBuilder.append(KGRN(label.toString()))
val shortLabel = formatName(shortName, theType)
val label = if (name == "") shortLabel else {
val longLabel = formatName(name, theType)
if (shortName.length > name.length) throw new IllegalArgumentException(s"Short name '$shortName' is longer than name '$name'")
shortLabel + ", " + longLabel
}
stringBuilder.append(KGRN(label))

// If the label is short enough, just pad out the column, otherwise wrap to the next line for the description
val numSpaces: Int = if (label.length > ArgumentColumnWidth) {
Expand Down
17 changes: 15 additions & 2 deletions sopt/src/main/scala/dagr/sopt/cmdline/ClpReflectiveBuilder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,21 @@ private[sopt] class ClpArgument(declaringClass: Class[_],

lazy val isSpecial: Boolean = annotation.exists(_.special())
lazy val isSensitive: Boolean = annotation.exists(_.sensitive())
lazy val longName: String = if (annotation.isDefined && annotation.get.name.nonEmpty) annotation.get.name else StringUtil.camelToGnu(name)
lazy val shortName: String = annotation.map(_.flag()).getOrElse("")
lazy val (shortName, longName) = {
val flagName = annotation.map(_.flag()).getOrElse("")
if (annotation.isDefined && annotation.get.name.nonEmpty) {
if (flagName.length > annotation.get.name.length) {
throw new IllegalStateException("The @arg flag (short name) should be shorter than the @arg name (long name)")
}
(flagName, annotation.get.name)
} else {
// It is ok that the length of the long name when derived from the field name is shorter than shortName since
// we will just swap the two.
val derivedName = StringUtil.camelToGnu(name)
if (derivedName == flagName) (flagName, "")
else if (flagName.length < derivedName.length) (flagName, derivedName) else (derivedName, flagName)
}
}
lazy val doc: String = annotation.map(_.doc()).getOrElse("")
lazy val isCommon: Boolean = annotation.exists(_.common())
lazy val minElements: Int = if (isCollection) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import org.scalatest.{BeforeAndAfterAll, Inside, OptionValues}

import scala.collection.Map
import scala.collection.immutable.HashMap
import scala.util.Success

////////////////////////////////////////////////////////////////////////////////
// Holds the classes we need for testing. These cannot be inner classes.
Expand Down Expand Up @@ -312,6 +311,66 @@ private case class BadEnumClass
)
private case class SomeDescription (@arg var a: Int = 2)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class MultiCharacterShortName
(
@arg(flag="atm") var automatedTellerMachine: String = "Bank"
)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class UseNameInAnnotation
(
@arg(name="atm") var automatedTellerMachine: String = "Bank"
)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class UseVariableName
(
@arg var automatedTellerMachine: String = "Bank"
)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class OneCharNameInAnnotation
(
@arg(name="a") var automatedTellerMachine: String = "Bank"
)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class OneCharVariableName
(
@arg var a: String = "Bank"
)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class FlagIsLongAndAnnotationNameIsShort
(
@arg(flag="long", name="a") var a: String = "Bank"
)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class FlagIsLongAndVariableNameIsShort
(
@arg(flag="long") var a: String = "Bank"
)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class FlagAndAnnotationNameAreA
(
@arg(flag="a", name="a") var a: String = "Bank"
)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class FlagAndVariableNameAreA
(
@arg(flag="a") var a: String = "Bank"
)

@clp(description = "", group = classOf[TestGroup], hidden = true)
private case class AnnotationNameIsCamelCase
(
@arg(name="camelCase") var a: String = "Bank"
)

////////////////////////////////////////////////////////////////////////////////
// End of Testing CLP classes
////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -576,8 +635,8 @@ with CommandLineParserStrings with CaptureSystemStreams with BeforeAndAfterAll {

"CommandLineProgramParser.usage" should "print out no arguments when no arguments are present" in {
val usage = parser(classOf[NoArguments]).usage(printCommon = false, withVersion = true, withSpecial = false)
usage should not include (RequiredArguments)
usage should not include (OptionalArguments)
usage should not include RequiredArguments
usage should not include OptionalArguments
usage should include (UsagePrefix)
}

Expand Down Expand Up @@ -672,7 +731,56 @@ with CommandLineParserStrings with CaptureSystemStreams with BeforeAndAfterAll {
val end = usage.indexOf("<END>")
start should be > 0
end should be > 0
usage.substring(start, end).split("\n").size shouldBe 4
usage.substring(start, end).split("\n").length shouldBe 4
}

it should "format a multi-character flag" in {
val usage = parser(classOf[MultiCharacterShortName]).usage(printCommon = false, withVersion = true, withSpecial = false)
usage should include ("--atm=String, --automated-teller-machine=String")
}

it should "format using the 'name' in the annotation for the long name when specified" in {
val usage = parser(classOf[UseNameInAnnotation]).usage(printCommon = false, withVersion = true, withSpecial = false)
usage should include ("--atm=String")
usage should not include "--automated-teller-machine"
}

it should "format using the variable name for the long name when 'name' is not specified" in {
val usage = parser(classOf[UseVariableName]).usage(printCommon = false, withVersion = true, withSpecial = false)
usage should include ("--automated-teller-machine=String")
}

it should "format using the single-character 'name' in the annotation for the long name when 'name' is specified" in {
val usage = parser(classOf[OneCharNameInAnnotation]).usage(printCommon = false, withVersion = true, withSpecial = false)
usage should include ("-a String")
}

it should "format using the single-character variable name for the long name when 'name' is not specified" in {
val usage = parser(classOf[OneCharVariableName]).usage(printCommon = false, withVersion = true, withSpecial = false)
usage should include ("-a String")
}

it should "throw an exception when the length of 'flag' is greater than the length of 'name'" in {
an[Exception] should be thrownBy parser(classOf[FlagIsLongAndAnnotationNameIsShort]).usage(printCommon = false, withVersion = true, withSpecial = false)
}

it should "format short arguments before long arguments, when the length of 'flag' is longer than 'name' is derived from the field's name" in {
val usage = parser(classOf[FlagIsLongAndVariableNameIsShort]).usage(printCommon = false, withVersion = true, withSpecial = false)
usage should include ("-a String, --long=String")
}

it should "throw an exception when the values for 'flag' and 'name' are the same" in {
an[Exception] should be thrownBy parser(classOf[FlagAndAnnotationNameAreA]).usage(printCommon = false, withVersion = true, withSpecial = false)
}

it should "allow camel casing in the 'name' field" in {
val usage = parser(classOf[AnnotationNameIsCamelCase]).usage(printCommon = false, withVersion = true, withSpecial = false)
usage should include ("--camelCase=String")
}

it should "allow the flag to be specified as the same as 'name' when 'name' is derived from the field's name " in {
val usage = parser(classOf[FlagAndVariableNameAreA]).usage(printCommon = false, withVersion = true, withSpecial = false)
usage should include ("-a String")
}

"CommandLineProgramParser.parseAndBuild" should "parse multiple positional arguments" in {
Expand Down Expand Up @@ -1015,8 +1123,15 @@ with CommandLineParserStrings with CaptureSystemStreams with BeforeAndAfterAll {
}
}

it should "parse a flag (short name) that is more than one character long" in {
val args = Array[String]("--atm", "Vault")
val p = parser(classOf[MultiCharacterShortName])
inside (p.parseAndBuild(args=args)) { case ParseSuccess() => }
val task = p.instance.get
task.automatedTellerMachine shouldBe "Vault"
}

"CommandLineProgramParser.getCommandLine" should "should return the command line string" in {
val task = new GeneralTestingProgram
val args = Array[String](
"--string-set", "Foo", "Bar",
"--int-set", "1", "2",
Expand Down

0 comments on commit 81db51b

Please sign in to comment.