Skip to content

Commit 00a8677

Browse files
authored
Clean up help generation (#385)
- Adds check to ensure that `wrapped(to:wrappingIndent:)` doesn't attempt to retrieve a negative prefix. - The Usage struct was composed of an array of strings which always contained exact one string at runtime. This struct has been removed and replaced with a single usage string. - Removes HelpGenerator._screenWidthOverride in favor of explicitly setting the screen width in generateHelp calls.
1 parent df75d70 commit 00a8677

File tree

4 files changed

+26
-35
lines changed

4 files changed

+26
-35
lines changed

Sources/ArgumentParser/Usage/HelpCommand.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,10 @@ struct HelpCommand: ParsableCommand {
3333
mutating func buildCommandStack(with parser: CommandParser) throws {
3434
commandStack = parser.commandStack(for: subcommands)
3535
}
36-
37-
func generateHelp() -> String {
38-
return HelpGenerator(commandStack: commandStack).rendered()
36+
37+
/// Used for testing.
38+
func generateHelp(screenWidth: Int) -> String {
39+
HelpGenerator(commandStack: commandStack).rendered(screenWidth: screenWidth)
3940
}
4041

4142
enum CodingKeys: CodingKey {

Sources/ArgumentParser/Usage/HelpGenerator.swift

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,8 @@
1212
internal struct HelpGenerator {
1313
static var helpIndent = 2
1414
static var labelColumnWidth = 26
15-
static var systemScreenWidth: Int {
16-
_screenWidthOverride ?? _terminalSize().width
17-
}
18-
19-
internal static var _screenWidthOverride: Int? = nil
20-
21-
struct Usage {
22-
var components: [String]
23-
24-
func rendered(screenWidth: Int) -> String {
25-
components
26-
.joined(separator: "\n")
27-
}
28-
}
29-
15+
static var systemScreenWidth: Int { _terminalSize().width }
16+
3017
struct Section {
3118
struct Element: Hashable {
3219
var label: String
@@ -98,7 +85,7 @@ internal struct HelpGenerator {
9885

9986
var commandStack: [ParsableCommand.Type]
10087
var abstract: String
101-
var usage: Usage
88+
var usage: String
10289
var sections: [Section]
10390
var discussionSections: [DiscussionSection]
10491

@@ -116,10 +103,10 @@ internal struct HelpGenerator {
116103
toolName = "\(superName) \(toolName)"
117104
}
118105

119-
var usageString = UsageGenerator(toolName: toolName, definition: [currentArgSet]).synopsis
106+
var usage = UsageGenerator(toolName: toolName, definition: [currentArgSet]).synopsis
120107
if !currentCommand.configuration.subcommands.isEmpty {
121-
if usageString.last != " " { usageString += " " }
122-
usageString += "<subcommand>"
108+
if usage.last != " " { usage += " " }
109+
usage += "<subcommand>"
123110
}
124111

125112
self.abstract = currentCommand.configuration.abstract
@@ -130,7 +117,7 @@ internal struct HelpGenerator {
130117
self.abstract += "\n\(currentCommand.configuration.discussion)"
131118
}
132119

133-
self.usage = Usage(components: [usageString])
120+
self.usage = usage
134121
self.sections = HelpGenerator.generateSections(commandStack: commandStack)
135122
self.discussionSections = []
136123
}
@@ -222,9 +209,8 @@ internal struct HelpGenerator {
222209
]
223210
}
224211

225-
func usageMessage(screenWidth: Int? = nil) -> String {
226-
let screenWidth = screenWidth ?? HelpGenerator.systemScreenWidth
227-
return "Usage: \(usage.rendered(screenWidth: screenWidth))"
212+
func usageMessage() -> String {
213+
return "Usage: \(usage)"
228214
}
229215

230216
var includesSubcommands: Bool {
@@ -243,7 +229,7 @@ internal struct HelpGenerator {
243229
? ""
244230
: "OVERVIEW: \(abstract)".wrapped(to: screenWidth) + "\n\n"
245231

246-
var helpSubcommandMessage: String = ""
232+
var helpSubcommandMessage = ""
247233
if includesSubcommands {
248234
var names = commandStack.map { $0._commandName }
249235
if let superName = commandStack.first!.configuration._superCommandName {
@@ -259,7 +245,7 @@ internal struct HelpGenerator {
259245

260246
return """
261247
\(renderedAbstract)\
262-
USAGE: \(usage.rendered(screenWidth: screenWidth))
248+
USAGE: \(usage)
263249
264250
\(renderedSections)\(helpSubcommandMessage)
265251
"""

Sources/ArgumentParser/Utilities/StringExtensions.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@
1212
extension String {
1313
func wrapped(to columns: Int, wrappingIndent: Int = 0) -> String {
1414
let columns = columns - wrappingIndent
15+
guard columns > 0 else {
16+
// Skip wrapping logic if the number of columns is less than 1 in release
17+
// builds and assert in debug builds.
18+
assertionFailure("`columns - wrappingIndent` should be always be greater than 0.")
19+
return ""
20+
}
21+
1522
var result: [Substring] = []
1623

1724
var currentIndex = startIndex

Tests/ArgumentParserPackageManagerTests/HelpTests.swift

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ func getErrorText<T: ParsableArguments>(_: T.Type, _ arguments: [String]) -> Str
2626
}
2727
}
2828

29-
func getErrorText<T: ParsableCommand>(_: T.Type, _ arguments: [String]) -> String {
29+
func getErrorText<T: ParsableCommand>(_: T.Type, _ arguments: [String], screenWidth: Int) -> String {
3030
do {
3131
let command = try T.parseAsRoot(arguments)
3232
if let helpCommand = command as? HelpCommand {
33-
return helpCommand.generateHelp()
33+
return helpCommand.generateHelp(screenWidth: screenWidth)
3434
} else {
3535
XCTFail("Didn't generate a help error")
3636
return ""
@@ -90,7 +90,7 @@ extension HelpTests {
9090

9191
func testConfigHelp() throws {
9292
XCTAssertEqual(
93-
getErrorText(Package.self, ["help", "config"]).trimmingLines(),
93+
getErrorText(Package.self, ["help", "config"], screenWidth: 80).trimmingLines(),
9494
"""
9595
USAGE: package config <subcommand>
9696
@@ -107,11 +107,8 @@ extension HelpTests {
107107
}
108108

109109
func testGetMirrorHelp() throws {
110-
HelpGenerator._screenWidthOverride = 80
111-
defer { HelpGenerator._screenWidthOverride = nil }
112-
113110
XCTAssertEqual(
114-
getErrorText(Package.self, ["help", "config", "get-mirror"]).trimmingLines(),
111+
getErrorText(Package.self, ["help", "config", "get-mirror"], screenWidth: 80).trimmingLines(),
115112
"""
116113
USAGE: package config get-mirror [<options>] --package-url <package-url>
117114

0 commit comments

Comments
 (0)