Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add progress step component #139

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/conventional-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@ jobs:
- uses: CondeNast/conventional-pull-request-action@v0.2.0
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
commitTitleMatch: false
Comment on lines +19 to +20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be kept true – otherwise, it's easy to use the wrong message when there's just one commit

27 changes: 0 additions & 27 deletions Package.resolved
Original file line number Diff line number Diff line change
@@ -1,23 +1,5 @@
{
"pins" : [
{
"identity" : "asynchrone",
"kind" : "remoteSourceControl",
"location" : "https://github.com/reddavis/Asynchrone",
"state" : {
"revision" : "1ddfcd3bc93277f68dffb793fc60001902f2517b",
"version" : "0.22.0"
}
},
{
"identity" : "combinex",
"kind" : "remoteSourceControl",
"location" : "https://github.com/cx-org/CombineX",
"state" : {
"revision" : "98096c6b2a51481cb6e4bae8da0a808d8cab09a1",
"version" : "0.4.0"
}
},
{
"identity" : "rainbow",
"kind" : "remoteSourceControl",
Expand All @@ -35,15 +17,6 @@
"revision" : "41982a3656a71c768319979febd796c6fd111d5c",
"version" : "1.5.0"
}
},
{
"identity" : "swift-atomics",
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-atomics.git",
"state" : {
"revision" : "3e95ba32cd1b4c877f6163e8eea54afc4e63bf9f",
"version" : "0.0.3"
}
}
],
"version" : 2
Expand Down
4 changes: 0 additions & 4 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ let package = Package(
],
dependencies: [
.package(url: "https://github.com/onevcat/Rainbow", .upToNextMajor(from: "4.0.1")),
.package(url: "https://github.com/cx-org/CombineX", .upToNextMajor(from: "0.4.0")),
.package(url: "https://github.com/reddavis/Asynchrone", .upToNextMajor(from: "0.22.0")),
.package(url: "https://github.com/apple/swift-argument-parser", .upToNextMajor(from: "1.5.0")),
],
targets: [
Expand All @@ -32,8 +30,6 @@ let package = Package(
name: "Noora",
dependencies: [
"Rainbow",
"CombineX",
"Asynchrone",
],
swiftSettings: [
.define("MOCKING", .when(configuration: .debug)),
Expand Down
153 changes: 153 additions & 0 deletions Sources/Noora/Components/ProgressStep.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
import Foundation
import Rainbow

class ProgressStep {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be a class?

// MARK: - Attributes

let message: String
let successMessage: String?
let errorMessage: String?
let showSpinner: Bool
let action: (@escaping (String) -> Void) async throws -> Void
let theme: Theme
let terminal: Terminaling
let renderer: Rendering
let standardPipelines: StandardPipelines
var spinner: Spinning
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a var?


init(
message: String,
successMessage: String?,
errorMessage: String?,
showSpinner: Bool,
action: @escaping (@escaping (String) -> Void) async throws -> Void,
theme: Theme,
Comment on lines +19 to +24
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep the init for dependency injection and pass the rest of the variables through the run invocation

terminal: Terminaling,
renderer: Rendering,
standardPipelines: StandardPipelines,
spinner: Spinning = Spinner()
) {
self.message = message
self.successMessage = successMessage
self.errorMessage = errorMessage
self.showSpinner = showSpinner
self.action = action
self.theme = theme
self.terminal = terminal
self.renderer = renderer
self.standardPipelines = standardPipelines
self.spinner = spinner
}

func run() async throws {
if terminal.isInteractive {
try await runInteractive()
} else {
try await runNonInteractive()
}
}

func runNonInteractive() async throws {
/// ℹ︎
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// ℹ︎

let start = DispatchTime.now()

// swiftlint:disable:next identifier_name
var _error: Error?

do {
standardPipelines.output.write(content: "\("ℹ︎".hexIfColoredTerminal(theme.primary, terminal)) \(message)\n")

try await action { progressMessage in
self.standardPipelines.output
.write(content: " \(progressMessage.hexIfColoredTerminal(self.theme.muted, self.terminal))\n")
}
} catch {
_error = error
}

let elapsedTime = Double(DispatchTime.now().uptimeNanoseconds - start.uptimeNanoseconds) / 1_000_000_000
let timeString = "[\(String(format: "%.1f", elapsedTime))s]".hexIfColoredTerminal(theme.muted, terminal)

if _error != nil {
standardPipelines.error
.write(
content: " \("⨯".hexIfColoredTerminal(theme.danger, terminal)) \((errorMessage ?? message).hexIfColoredTerminal(theme.muted, terminal)) \(timeString)\n"
)
Comment on lines +72 to +75
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if static errorMessage is the best approach here.
What do you think about passing a callback in the form of (Error) -> String to transform Error into a String? But since we rethrow, it might be also fine as I'd expect the consumers to deal with that error next on their own side.

} else {
let message = ProgressStep
.completionMessage(successMessage ?? message, timeString: timeString, theme: theme, terminal: terminal)
standardPipelines.output.write(content: " \(message)\n")
}

// swiftlint:disable:next identifier_name
if let _error {
throw _error
}
Comment on lines +63 to +85
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd extract the following two lines into a helped method/closure:

private func timeString(start: DispatchTime) -> String {
        let elapsedTime = Double(DispatchTime.now().uptimeNanoseconds - start.uptimeNanoseconds) / 1_000_000_000
        return "[\(String(format: "%.1f", elapsedTime))s]".hexIfColoredTerminal(theme.muted, terminal)
}

That way, we can save ourselves some of the dancing with _error

Suggested change
}
} catch {
_error = error
}
let elapsedTime = Double(DispatchTime.now().uptimeNanoseconds - start.uptimeNanoseconds) / 1_000_000_000
let timeString = "[\(String(format: "%.1f", elapsedTime))s]".hexIfColoredTerminal(theme.muted, terminal)
if _error != nil {
standardPipelines.error
.write(
content: " \("".hexIfColoredTerminal(theme.danger, terminal)) \((errorMessage ?? message).hexIfColoredTerminal(theme.muted, terminal)) \(timeString)\n"
)
} else {
let message = ProgressStep
.completionMessage(successMessage ?? message, timeString: timeString, theme: theme, terminal: terminal)
standardPipelines.output.write(content: " \(message)\n")
}
// swiftlint:disable:next identifier_name
if let _error {
throw _error
}
}
let message = ProgressStep
.completionMessage(successMessage ?? message, timeString: timeString(start: start), theme: theme, terminal: terminal)
standardPipelines.output.write(content: " \(message)\n")
} catch {
standardPipelines.error
.write(
content: " \("".hexIfColoredTerminal(theme.danger, terminal)) \((errorMessage ?? message).hexIfColoredTerminal(theme.muted, terminal)) \(timeString(start: start))\n"
)
throw error
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same goes for runInteractive – we can make the code more readable if we skip doing the _error dance.

}

func runInteractive() async throws {
let start = DispatchTime.now()

defer {
if showSpinner {
spinner.stop()
}
}

var spinnerIcon: String?
var lastMessage = message

if showSpinner {
spinner.spin { icon in
spinnerIcon = icon
self.render(message: lastMessage, icon: spinnerIcon ?? "ℹ︎")
}
}

// swiftlint:disable:next identifier_name
var _error: Error?
do {
render(message: lastMessage, icon: spinnerIcon ?? "ℹ︎")
try await action { progressMessage in
lastMessage = progressMessage
self.render(message: lastMessage, icon: spinnerIcon ?? "ℹ︎")
}
} catch {
_error = error
}

let elapsedTime = Double(DispatchTime.now().uptimeNanoseconds - start.uptimeNanoseconds) / 1_000_000_000
let timeString = "[\(String(format: "%.1f", elapsedTime))s]".hexIfColoredTerminal(theme.muted, terminal)

if _error != nil {
renderer.render(
"\("⨯".hexIfColoredTerminal(theme.danger, terminal)) \(errorMessage ?? message) \(timeString)",
standardPipeline: standardPipelines.error
)
} else {
renderer.render(
ProgressStep
.completionMessage(successMessage ?? message, timeString: timeString, theme: theme, terminal: terminal),
standardPipeline: standardPipelines.output
)
}

// swiftlint:disable:next identifier_name
if let _error {
throw _error
}
}

// MARK: - Private
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The completionMessage is not private. It should be moved up. FWIW, I'm not a huge fan of the MARK sections and I'd use them sparingly. They often get out-of-date and they don't bring a ton of value (for me, anyways).


static func completionMessage(_ message: String, timeString: String? = nil, theme: Theme, terminal: Terminaling) -> String {
"\("✔︎".hexIfColoredTerminal(theme.success, terminal)) \(message)\(timeString != nil ? " \(timeString!)" : "")"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"\("✔︎".hexIfColoredTerminal(theme.success, terminal)) \(message)\(timeString != nil ? " \(timeString!)" : "")"
"\("✔︎".hexIfColoredTerminal(theme.success, terminal)) \(message)\(timeString ?? "")"

}

private func render(message: String, icon: String) {
renderer.render(
"\(icon.hexIfColoredTerminal(theme.primary, terminal)) \(message)",
standardPipeline: standardPipelines.output
)
}
}
5 changes: 4 additions & 1 deletion Sources/Noora/Components/SingleChoicePrompt.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ struct SingleChoicePrompt<T: CaseIterable & CustomStringConvertible & Equatable>
.boldIfColoredTerminal(terminal)
}
content += " \(selectedOption.description)"
renderer.render(content, standardPipeline: standardPipelines.output)
renderer.render(
ProgressStep.completionMessage(content, theme: theme, terminal: terminal),
standardPipeline: standardPipelines.output
)
}

private func renderOptions(selectedOption: T) {
Expand Down
6 changes: 5 additions & 1 deletion Sources/Noora/Components/YesOrNoChoicePrompt.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ struct YesOrNoChoicePrompt {
.boldIfColoredTerminal(terminal)
}
content += " \(answer ? "Yes" : "No")"
renderer.render(content, standardPipeline: standardPipelines.output)

renderer.render(
ProgressStep.completionMessage(content, theme: theme, terminal: terminal),
standardPipeline: standardPipelines.output
)
}

private func renderOptions(answer: Bool) {
Expand Down
21 changes: 21 additions & 0 deletions Sources/Noora/Noora.swift
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,25 @@ public struct Noora: Noorable {
theme: theme
).run()
}

public func progressStep(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is not part of the Noorable protocol. The method is missing documentation (which should be in the Noorable definition)

message: String,
successMessage: String? = nil,
errorMessage: String? = nil,
showSpinner: Bool = true,
action: @escaping ((String) -> Void) async throws -> Void
) async throws {
let progressStep = ProgressStep(
message: message,
successMessage: successMessage,
errorMessage: errorMessage,
showSpinner: showSpinner,
action: action,
theme: theme,
terminal: terminal,
renderer: Renderer(),
standardPipelines: StandardPipelines()
)
try await progressStep.run()
}
}
52 changes: 29 additions & 23 deletions Sources/Noora/Utilities/Spinner.swift
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
import CombineX
import CXFoundation
import Foundation

enum Spinner {
typealias Cancellable = () -> Void

actor Counter {
var count: Int = 0

func increase() {
count += 1
}
}
protocol Spinning {
func spin(_ block: @escaping (String) -> Void)
func stop()
}

class Spinner: Spinning {
private static let frames = [
"⠋",
"⠙",
Expand All @@ -25,21 +18,34 @@ enum Spinner {
"⠇",
"⠏",
]
private var isSpinning = true
private var timer: Timer?

static func spin(_ block: @escaping (String) async -> Void) async -> Cancellable {
let counter = Counter()
await block(Spinner.frames[0])
func spin(_ block: @escaping (String) -> Void) {
isSpinning = true

let cancellable = Timer.CX.TimerPublisher(interval: 0.1, runLoop: .main, mode: .common)
.autoconnect()
.sink { _ in
Task {
await block(Spinner.frames[await counter.count % Spinner.frames.count])
await counter.increase()
DispatchQueue.global(qos: .userInitiated).async {
let runLoop = RunLoop.current
var index = 0

// Schedule the timer in the current run loop
self.timer = Timer.scheduledTimer(withTimeInterval: 0.1, repeats: true) { _ in
if self.isSpinning {
block(Spinner.frames[index])
index = (index + 1) % Spinner.frames.count
} else {
self.timer?.invalidate()
}
}
return {
cancellable.cancel()

// Start the run loop to allow the timer to fire
while self.isSpinning, runLoop.run(mode: .default, before: .distantFuture) {}
}
}

func stop() {
isSpinning = false
timer?.invalidate()
timer = nil
}
}
34 changes: 34 additions & 0 deletions Sources/examples-cli/Commands/ProgressStepCommand.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import ArgumentParser
import Foundation
import Noora

struct ProgressStepCommand: AsyncParsableCommand {
static let configuration = CommandConfiguration(
commandName: "progress-step",
abstract: "A component to shows a progress step"
)

func run() async throws {
try await Noora().progressStep(
message: "Loading manifests",
successMessage: "Manifests loaded",
errorMessage: "Failed to load manifests"
) { _ in
try await Task.sleep(nanoseconds: 2_000_000_000)
}
try await Noora().progressStep(
message: "Processing the graph",
successMessage: "Project graph processed",
errorMessage: "Failed to process the project graph"
) { _ in
try await Task.sleep(nanoseconds: 2_000_000_000)
}
try await Noora().progressStep(
message: "Generating Xcode projects and workspace",
successMessage: "Xcode projects and workspace generated",
errorMessage: "Failed to generate Xcode workspace and projects"
) { _ in
try await Task.sleep(nanoseconds: 2_000_000_000)
}
}
}
7 changes: 6 additions & 1 deletion Sources/examples-cli/ExamplesCLI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import Rainbow
struct ExamplesCLI: AsyncParsableCommand {
static let configuration = CommandConfiguration(
abstract: "A command line tool to test the different components available in Noora.",
subcommands: [SingleChoicePromptCommand.self, YesOrNoChoicePromptCommand.self, AlertCommand.self]
subcommands: [
SingleChoicePromptCommand.self,
YesOrNoChoicePromptCommand.self,
AlertCommand.self,
ProgressStepCommand.self,
]
)
}
Loading
Loading