From 00a86771caa3cd1ff4db09f4a2bd28c70fdc54cc Mon Sep 17 00:00:00 2001 From: Rauhul Varma Date: Mon, 10 Jan 2022 14:02:50 -0800 Subject: [PATCH] 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. --- .../ArgumentParser/Usage/HelpCommand.swift | 7 ++-- .../ArgumentParser/Usage/HelpGenerator.swift | 36 ++++++------------- .../Utilities/StringExtensions.swift | 7 ++++ .../HelpTests.swift | 11 +++--- 4 files changed, 26 insertions(+), 35 deletions(-) diff --git a/Sources/ArgumentParser/Usage/HelpCommand.swift b/Sources/ArgumentParser/Usage/HelpCommand.swift index 272b74a9..5e411fa7 100644 --- a/Sources/ArgumentParser/Usage/HelpCommand.swift +++ b/Sources/ArgumentParser/Usage/HelpCommand.swift @@ -33,9 +33,10 @@ struct HelpCommand: ParsableCommand { mutating func buildCommandStack(with parser: CommandParser) throws { commandStack = parser.commandStack(for: subcommands) } - - func generateHelp() -> String { - return HelpGenerator(commandStack: commandStack).rendered() + + /// Used for testing. + func generateHelp(screenWidth: Int) -> String { + HelpGenerator(commandStack: commandStack).rendered(screenWidth: screenWidth) } enum CodingKeys: CodingKey { diff --git a/Sources/ArgumentParser/Usage/HelpGenerator.swift b/Sources/ArgumentParser/Usage/HelpGenerator.swift index e7f8e8f8..87edfb55 100644 --- a/Sources/ArgumentParser/Usage/HelpGenerator.swift +++ b/Sources/ArgumentParser/Usage/HelpGenerator.swift @@ -12,21 +12,8 @@ internal struct HelpGenerator { static var helpIndent = 2 static var labelColumnWidth = 26 - static var systemScreenWidth: Int { - _screenWidthOverride ?? _terminalSize().width - } - - internal static var _screenWidthOverride: Int? = nil - - struct Usage { - var components: [String] - - func rendered(screenWidth: Int) -> String { - components - .joined(separator: "\n") - } - } - + static var systemScreenWidth: Int { _terminalSize().width } + struct Section { struct Element: Hashable { var label: String @@ -98,7 +85,7 @@ internal struct HelpGenerator { var commandStack: [ParsableCommand.Type] var abstract: String - var usage: Usage + var usage: String var sections: [Section] var discussionSections: [DiscussionSection] @@ -116,10 +103,10 @@ internal struct HelpGenerator { toolName = "\(superName) \(toolName)" } - var usageString = UsageGenerator(toolName: toolName, definition: [currentArgSet]).synopsis + var usage = UsageGenerator(toolName: toolName, definition: [currentArgSet]).synopsis if !currentCommand.configuration.subcommands.isEmpty { - if usageString.last != " " { usageString += " " } - usageString += "" + if usage.last != " " { usage += " " } + usage += "" } self.abstract = currentCommand.configuration.abstract @@ -130,7 +117,7 @@ internal struct HelpGenerator { self.abstract += "\n\(currentCommand.configuration.discussion)" } - self.usage = Usage(components: [usageString]) + self.usage = usage self.sections = HelpGenerator.generateSections(commandStack: commandStack) self.discussionSections = [] } @@ -222,9 +209,8 @@ internal struct HelpGenerator { ] } - func usageMessage(screenWidth: Int? = nil) -> String { - let screenWidth = screenWidth ?? HelpGenerator.systemScreenWidth - return "Usage: \(usage.rendered(screenWidth: screenWidth))" + func usageMessage() -> String { + return "Usage: \(usage)" } var includesSubcommands: Bool { @@ -243,7 +229,7 @@ internal struct HelpGenerator { ? "" : "OVERVIEW: \(abstract)".wrapped(to: screenWidth) + "\n\n" - var helpSubcommandMessage: String = "" + var helpSubcommandMessage = "" if includesSubcommands { var names = commandStack.map { $0._commandName } if let superName = commandStack.first!.configuration._superCommandName { @@ -259,7 +245,7 @@ internal struct HelpGenerator { return """ \(renderedAbstract)\ - USAGE: \(usage.rendered(screenWidth: screenWidth)) + USAGE: \(usage) \(renderedSections)\(helpSubcommandMessage) """ diff --git a/Sources/ArgumentParser/Utilities/StringExtensions.swift b/Sources/ArgumentParser/Utilities/StringExtensions.swift index c402d3e5..a6a8e83f 100644 --- a/Sources/ArgumentParser/Utilities/StringExtensions.swift +++ b/Sources/ArgumentParser/Utilities/StringExtensions.swift @@ -12,6 +12,13 @@ extension String { func wrapped(to columns: Int, wrappingIndent: Int = 0) -> String { let columns = columns - wrappingIndent + guard columns > 0 else { + // Skip wrapping logic if the number of columns is less than 1 in release + // builds and assert in debug builds. + assertionFailure("`columns - wrappingIndent` should be always be greater than 0.") + return "" + } + var result: [Substring] = [] var currentIndex = startIndex diff --git a/Tests/ArgumentParserPackageManagerTests/HelpTests.swift b/Tests/ArgumentParserPackageManagerTests/HelpTests.swift index c2aca354..8e30881d 100644 --- a/Tests/ArgumentParserPackageManagerTests/HelpTests.swift +++ b/Tests/ArgumentParserPackageManagerTests/HelpTests.swift @@ -26,11 +26,11 @@ func getErrorText(_: T.Type, _ arguments: [String]) -> Str } } -func getErrorText(_: T.Type, _ arguments: [String]) -> String { +func getErrorText(_: T.Type, _ arguments: [String], screenWidth: Int) -> String { do { let command = try T.parseAsRoot(arguments) if let helpCommand = command as? HelpCommand { - return helpCommand.generateHelp() + return helpCommand.generateHelp(screenWidth: screenWidth) } else { XCTFail("Didn't generate a help error") return "" @@ -90,7 +90,7 @@ extension HelpTests { func testConfigHelp() throws { XCTAssertEqual( - getErrorText(Package.self, ["help", "config"]).trimmingLines(), + getErrorText(Package.self, ["help", "config"], screenWidth: 80).trimmingLines(), """ USAGE: package config @@ -107,11 +107,8 @@ extension HelpTests { } func testGetMirrorHelp() throws { - HelpGenerator._screenWidthOverride = 80 - defer { HelpGenerator._screenWidthOverride = nil } - XCTAssertEqual( - getErrorText(Package.self, ["help", "config", "get-mirror"]).trimmingLines(), + getErrorText(Package.self, ["help", "config", "get-mirror"], screenWidth: 80).trimmingLines(), """ USAGE: package config get-mirror [] --package-url