Use the first long name in usage (#179)

* Use the first long name in usage (#167)

Sorting names for option has been changed.
* Usage will choose the first long name, if available, or otherwise the first short name
* Help screen will show short names, then long names, in the order of their declaration.

Solves (#167)

* Fix adding same name more than once.

Checkin long names in sort caused single dash long names to be treated as a short name.
Fixed sort to check “short” instead of “long”

Update current test to cover more cases.

* Add isShort control to Name

*Replace sortedNames with partitionedNames

*Extract uniquifying logic into SequenceExtensions.swift

* Refactor short test with computed property

* Use pattern-matching instead of equality comparison

Author: ibrahimoktay

Sources/ArgumentParser/Parsable Properties/NameSpecification.swift

@@ -38,10 +38,10 @@ public struct NameSpecification: ExpressibleByArrayLiteral {
     /// Short labels can be combined into groups.
     case customShort(Character)
   }
-  var elements: Set<Element>
+  var elements: Array<Element>
 
   public init<S>(_ sequence: S) where S : Sequence, Element == S.Element {
-    self.elements = Set(sequence)
+    self.elements = sequence.uniquified()
   }
 
   public init(arrayLiteral elements: Element...) {

Sources/ArgumentParser/Parsing/Name.swift

@@ -53,6 +53,15 @@ extension Name {
       return n
     }
   }
+  
+  var isShort: Bool {
+    switch self {
+    case .short:
+      return true
+    default:
+      return false
+    }
+  }
 }
 
 // short argument names based on the synopsisString

Sources/ArgumentParser/Usage/UsageGenerator.swift

@@ -63,7 +63,7 @@ extension ArgumentDefinition {
 
     switch kind {
     case .named:
-      let joinedSynopsisString = sortedNames
+      let joinedSynopsisString = partitionedNames
         .map { $0.synopsisString }
         .joined(separator: ", ")
 
@@ -113,30 +113,12 @@ extension ArgumentDefinition {
     return unadornedSynopsis
   }
 
-  var sortedNames: [Name] {
-    return names
-      .sorted { (lhs, rhs) -> Bool in
-        switch (lhs, rhs) {
-        case let (.long(l), .long(r)):
-          return l < r
-        case (_, .long):
-          return true
-        case (.long, _):
-          return false
-        case let (.short(l), .short(r)):
-          return l < r
-        case (_, .short):
-          return true
-        case (.short, _):
-          return false
-        case let (.longWithSingleDash(l), .longWithSingleDash(r)):
-          return l < r
-        }
-    }
+  var partitionedNames: [Name] {
+    return names.filter{ $0.isShort } + names.filter{ !$0.isShort }
   }
 
   var preferredNameForSynopsis: Name? {
-    sortedNames.last
+    names.first{ !$0.isShort } ?? names.first
   }
 
   var synopsisValueName: String? {

Sources/ArgumentParser/Utilities/SequenceExtensions.swift

@@ -0,0 +1,22 @@
+//===----------------------------------------------------------*- swift -*-===//
+//
+// This source file is part of the Swift Argument Parser open source project
+//
+// Copyright (c) 2020 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+//
+//===----------------------------------------------------------------------===//
+
+extension Sequence where Element: Equatable {
+  func uniquified() -> [Element] {
+    var sequence = Array<Element>()
+    for element in self {
+      if !sequence.contains(element) {
+        sequence.append(element)
+      }
+    }
+    return sequence
+  }
+}

Tests/ArgumentParserUnitTests/HelpGenerationTests.swift

@@ -358,4 +358,23 @@ extension HelpGenerationTests {
 
     """)
   }
+  
+  struct L: ParsableArguments {
+    @Option(
+      name: [.short, .customLong("remote"), .customLong("remote"), .short, .customLong("when"), .long, .customLong("other", withSingleDash: true), .customLong("there"), .customShort("x"), .customShort("y")],
+      help: "Help Message")
+    var time: String?
+  }
+  
+  func testHelpWithMultipleCustomNames() {
+    AssertHelp(for: L.self, equals: """
+    USAGE: l [--remote <remote>]
+
+    OPTIONS:
+      -t, -x, -y, --remote, --when, --time, -other, --there <remote>
+                              Help Message
+      -h, --help              Show help information.
+
+    """)
+  }
 }

Tests/ArgumentParserUnitTests/UsageGenerationTests.swift

@@ -149,4 +149,28 @@ extension UsageGenerationTests {
     let help = UsageGenerator(toolName: "bar", parsable: J())
     XCTAssertEqual(help.synopsis, "bar --req <req> [--opt <opt>]")
   }
+  
+  struct K: ParsableArguments {
+    @Option(
+      name: [.short, .customLong("remote"),  .customLong("when"), .customLong("there")],
+      help: "Help Message")
+    var time: String?
+  }
+  
+  func testSynopsisWithMultipleCustomNames() {
+    let help = UsageGenerator(toolName: "bar", parsable: K())
+    XCTAssertEqual(help.synopsis, "bar [--remote <remote>]")
+  }
+  
+  struct L: ParsableArguments {
+    @Option(
+      name: [.short, .short, .customLong("remote", withSingleDash: true), .short, .customLong("remote", withSingleDash: true)],
+      help: "Help Message")
+    var time: String?
+  }
+  
+  func testSynopsisWithSingleDashLongNameFirst() {
+    let help = UsageGenerator(toolName: "bar", parsable: L())
+    XCTAssertEqual(help.synopsis, "bar [-remote <remote>]")
+  }
 }