Simplify synopsis string generation

Sources/ArgumentParser/Completions/BashCompletionsGenerator.swift

@@ -145,7 +145,7 @@ struct BashCompletionsGenerator {
           let commandName = commands.first!._commandName
           let subcommandNames = commands.dropFirst().map { $0._commandName }.joined(separator: " ")
           // TODO: Make this work for @Arguments
-          let argumentName = arg.preferredNameForSynopsis?.synopsisString
+          let argumentName = arg.names.preferredName?.synopsisString
                 ?? arg.help.keys.first?.rawValue ?? "---"
 
           return """

Sources/ArgumentParser/Completions/CompletionsGenerator.swift

@@ -102,7 +102,7 @@ extension ArgumentDefinition {
   /// this argument.
   func customCompletionCall(_ commands: [ParsableCommand.Type]) -> String {
     let subcommandNames = commands.dropFirst().map { $0._commandName }.joined(separator: " ")
-    let argumentName = preferredNameForSynopsis?.synopsisString
+    let argumentName = names.preferredName?.synopsisString
           ?? self.help.keys.first?.rawValue ?? "---"
     return "---completion \(subcommandNames) -- \(argumentName)"
   }

Sources/ArgumentParser/Parsing/ArgumentDefinition.swift

@@ -115,7 +115,7 @@ struct ArgumentDefinition {
 
   var valueName: String {
     help.valueName.mapEmpty {
-      preferredNameForSynopsis?.valueString
+      names.preferredName?.valueString
         ?? help.keys.first?.rawValue.convertedToSnakeCase(separator: "-")
         ?? "value"
     }
@@ -165,7 +165,6 @@ extension ArgumentDefinition: CustomDebugStringConvertible {
 extension ArgumentDefinition {
   var optional: ArgumentDefinition {
     var result = self
-
     result.help.options.insert(.isOptional)
     return result
   }

Sources/ArgumentParser/Parsing/ArgumentSet.swift

@@ -159,15 +159,6 @@ extension ArgumentSet {
 extension ArgumentDefinition {
   /// Create a unary / argument that parses using the given closure.
   init<A>(key: InputKey, kind: ArgumentDefinition.Kind, parsingStrategy: ParsingStrategy = .default, parser: @escaping (String) -> A?, parseType type: A.Type = A.self, default initial: A?, completion: CompletionKind) {
-    let initialValueCreator: (InputOrigin, inout ParsedValues) throws -> Void
-    if let initialValue = initial {
-      initialValueCreator = { origin, values in
-        values.set(initialValue, forKey: key, inputOrigin: origin)
-      }
-    } else {
-      initialValueCreator = { _, _ in }
-    }
-
     self.init(kind: kind, help: ArgumentDefinition.Help(key: key), completion: completion, parsingStrategy: parsingStrategy, update: .unary({ (origin, name, value, values) in
       guard let v = parser(value) else {
         throw ParserError.unableToParseValue(origin, name, value, forKey: key)

Sources/ArgumentParser/Parsing/Name.swift

@@ -9,7 +9,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-enum Name: Hashable {
+enum Name {
   /// A name (usually multi-character) prefixed with `--` (2 dashes) or equivalent.
   case long(String)
   /// A single character name prefixed with `-` (1 dash) or equivalent.
@@ -18,7 +18,9 @@ enum Name: Hashable {
   case short(Character, allowingJoined: Bool = false)
   /// A name (usually multi-character) prefixed with `-` (1 dash).
   case longWithSingleDash(String)
-
+}
+
+extension Name {
   init(_ baseName: Substring) {
     assert(baseName.first == "-", "Attempted to create name for unprefixed argument")
     if baseName.hasPrefix("--") {
@@ -31,6 +33,35 @@ enum Name: Hashable {
   }
 }
 
+// short argument names based on the synopsisString
+// this will put the single - options before the -- options
+extension Name: Comparable {
+  static func < (lhs: Name, rhs: Name) -> Bool {
+    return lhs.synopsisString < rhs.synopsisString
+  }
+}
+
+extension Name: Hashable { }
+
+extension Name {
+  enum Case: Equatable {
+    case long
+    case short
+    case longWithSingleDash
+  }
+
+  var `case`: Case {
+    switch self {
+    case .short:
+      return .short
+    case .longWithSingleDash:
+      return .longWithSingleDash
+    case .long:
+      return .long
+    }
+  }
+}
+
 extension Name {
   var synopsisString: String {
     switch self {
@@ -53,16 +84,7 @@ extension Name {
       return n
     }
   }
-
-  var isShort: Bool {
-    switch self {
-    case .short:
-      return true
-    default:
-      return false
-    }
-  }
-
+
   var allowsJoined: Bool {
     switch self {
     case .short(_, let allowingJoined):
@@ -82,10 +104,12 @@ extension Name {
   }
 }
 
-// short argument names based on the synopsisString
-// this will put the single - options before the -- options
-extension Name: Comparable {
-  static func < (lhs: Name, rhs: Name) -> Bool {
-    return lhs.synopsisString < rhs.synopsisString
+extension BidirectionalCollection where Element == Name {
+  var preferredName: Name? {
+    first { $0.case != .short } ?? first
+  }
+
+  var paritioned: [Name] {
+    filter { $0.case == .short } + filter { $0.case != .short }
   }
 }

Sources/ArgumentParser/Usage/HelpGenerator.swift

@@ -161,10 +161,16 @@ internal struct HelpGenerator {
         let groupEnd = args.firstIndex(where: { $0.help.keys != arg.help.keys }) ?? args.endIndex
         let groupedArgs = [arg] + args[..<groupEnd]
         args = args[groupEnd...]
-
-        synopsis = groupedArgs.compactMap { $0.synopsisForHelp }.joined(separator: "/")
 
-        let defaultValue = arg.help.defaultValue.map { "(default: \($0))" } ?? ""
+        synopsis = groupedArgs
+          .lazy
+          .filter { $0.help.shouldDisplay }
+          .map { $0.synopsisForHelp }
+          .joined(separator: "/")
+
+        let defaultValue = arg.help.defaultValue
+          .map { "(default: \($0))" } ?? ""
+
         let descriptionString = groupedArgs
           .lazy
           .map { $0.help.abstract }
@@ -176,7 +182,9 @@ internal struct HelpGenerator {
           .filter { !$0.isEmpty }
           .joined(separator: " ")
       } else {
-        synopsis = arg.synopsisForHelp ?? ""
+        synopsis = arg.help.shouldDisplay
+          ? arg.synopsisForHelp
+          : ""
 
         let defaultValue = arg.help.defaultValue.flatMap { $0.isEmpty ? nil : "(default: \($0))" }
         description = [arg.help.abstract, defaultValue]
@@ -276,8 +284,7 @@ internal extension BidirectionalCollection where Element == ParsableCommand.Type
   }
 
   func getPrimaryHelpName() -> Name? {
-    let names = getHelpNames()
-    return names.first(where: { !$0.isShort }) ?? names.first
+    getHelpNames().preferredName
   }
 
   func versionArgumentDefintion() -> ArgumentDefinition? {

Sources/ArgumentParser/Usage/UsageGenerator.swift

@@ -36,49 +36,48 @@ extension UsageGenerator {
   ///
   /// In `roff`.
   var synopsis: String {
-    let definitionSynopsis = definition.synopsis
-    switch definitionSynopsis.count {
+    // Filter out options that should not be displayed.
+    var options = definition
+      .filter { $0.help.shouldDisplay }
+    switch options.count {
     case 0:
       return toolName
     case let x where x > 12:
       // When we have too many options, keep required and positional arguments,
       // but discard the rest.
-      let synopsis: [String] = definition.compactMap { argument in
-        guard argument.isPositional || !argument.help.options.contains(.isOptional) else {
-          return nil
-        }
-        return argument.synopsis
+      options = options.filter {
+        $0.isPositional || !$0.help.options.contains(.isOptional)
       }
-      if !synopsis.isEmpty, synopsis.count <= 12 {
-        return "\(toolName) [<options>] \(synopsis.joined(separator: " "))"
+      // If there are between 1 and 12 options left, print them, otherwise print
+      // a simplified usage string.
+      if !options.isEmpty, options.count <= 12 {
+        let synopsis = options
+          .map { $0.synopsis }
+          .joined(separator: " ")
+        return "\(toolName) [<options>] \(synopsis)"
       }
       return "\(toolName) <options>"
     default:
-      return "\(toolName) \(definition.synopsis.joined(separator: " "))"
+      let synopsis = options
+        .map { $0.synopsis }
+        .joined(separator: " ")
+      return "\(toolName) \(synopsis)"
     }
   }
 }
 
-extension ArgumentSet {
-  var synopsis: [String] {
-    return self
-      .compactMap { $0.synopsis }
-  }
-}
-
 extension ArgumentDefinition {
-  var synopsisForHelp: String? {
-    guard help.shouldDisplay else { return nil }
-
+  var synopsisForHelp: String {
     switch kind {
     case .named:
-      let joinedSynopsisString = partitionedNames
+      let joinedSynopsisString = names
+        .paritioned
         .map { $0.synopsisString }
         .joined(separator: ", ")
-      
+
       switch update {
       case .unary:
-        return "\(joinedSynopsisString) <\(synopsisValueName ?? "")>"
+        return "\(joinedSynopsisString) <\(valueName)>"
       case .nullary:
         return joinedSynopsisString
       }
@@ -88,15 +87,17 @@ extension ArgumentDefinition {
       return ""
     }
   }
-  
-  var unadornedSynopsis: String? {
+
+  var unadornedSynopsis: String {
     switch kind {
     case .named:
-      guard let name = preferredNameForSynopsis else { return nil }
-
+      guard let name = names.preferredName else {
+        fatalError("preferredName cannot be nil for named arguments")
+      }
+
       switch update {
       case .unary:
-        return "\(name.synopsisString) <\(synopsisValueName ?? "value")>"
+        return "\(name.synopsisString) <\(valueName)>"
       case .nullary:
         return name.synopsisString
       }
@@ -106,34 +107,16 @@ extension ArgumentDefinition {
       return ""
     }
   }
-
-  var synopsis: String? {
-    guard help.shouldDisplay else { return nil }
-
-    guard !help.options.contains(.isOptional) else {
-      var n = self
-      n.help.options.remove(.isOptional)
-      return n.synopsis.flatMap { "[\($0)]" }
+
+  var synopsis: String {
+    var synopsis = unadornedSynopsis
+    if help.options.contains(.isRepeating) {
+      synopsis += " ..."
     }
-    guard !help.options.contains(.isRepeating) else {
-      var n = self
-      n.help.options.remove(.isRepeating)
-      return n.synopsis.flatMap { "\($0) ..." }
+    if help.options.contains(.isOptional) {
+      synopsis = "[\(synopsis)]"
     }
-
-    return unadornedSynopsis
-  }
-
-  var partitionedNames: [Name] {
-    return names.filter{ $0.isShort } + names.filter{ !$0.isShort }
-  }
-
-  var preferredNameForSynopsis: Name? {
-    names.first { !$0.isShort } ?? names.first
-  }
-
-  var synopsisValueName: String? {
-    valueName
+    return synopsis
   }
 }
 
@@ -241,27 +224,20 @@ extension ErrorMessageGenerator {
 
 extension ErrorMessageGenerator {
   func arguments(for key: InputKey) -> [ArgumentDefinition] {
-    return arguments
-      .filter {
-        $0.help.keys.contains(key)
-    }
+    arguments
+      .filter { $0.help.keys.contains(key) }
   }
 
   func help(for key: InputKey) -> ArgumentDefinition.Help? {
-    return arguments
+    arguments
       .first { $0.help.keys.contains(key) }
       .map { $0.help }
   }
 
   func valueName(for name: Name) -> String? {
-    for arg in arguments {
-      guard
-        arg.names.contains(name),
-        let v = arg.synopsisValueName
-        else { continue }
-      return v
-    }
-    return nil
+    arguments
+      .first { $0.names.contains(name) }
+      .map { $0.valueName }
   }
 }
 
@@ -313,7 +289,7 @@ extension ErrorMessageGenerator {
       })
 
     if let suggestion = suggestion {
-        return "Unknown option '\(name.synopsisString)'. Did you mean '\(suggestion.synopsisString)'?"
+      return "Unknown option '\(name.synopsisString)'. Did you mean '\(suggestion.synopsisString)'?"
     }
     return "Unknown option '\(name.synopsisString)'"
   }
@@ -363,8 +339,10 @@ extension ErrorMessageGenerator {
 
   func noValueMessage(key: InputKey) -> String? {
     let args = arguments(for: key)
-    let possibilities = args.compactMap {
-      $0.nonOptional.synopsis
+    let possibilities: [String] = args.compactMap {
+      $0.help.shouldDisplay
+        ? $0.nonOptional.synopsis
+        : nil
     }
     switch possibilities.count {
     case 0: