Skip to content

Add subcommands - #182

Open
fpseverino wants to merge 27 commits into
vapor:vapor5from
fpseverino:subcommands
Open

fpseverino wants to merge 27 commits into
vapor:vapor5from
fpseverino:subcommands

Conversation

@fpseverino

@fpseverino fpseverino commented Oct 15, 2026 •

Copy link
Copy Markdown
Member
  • Add serve, routes and migrate subcommands (the latter is WIP) from Vapor 4 using Swift Argument Parser

Comment thread Sources/App/entrypoint.swift Outdated
var other: [String]

mutating func run() async throws {
print("🚧 Work in progress 🚧")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ptoffy should already have a fix ready for this

@0xTim 0xTim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's put each command into it's own file.

Also, just noticed that we should make secret configuration options as secrets

Comment thread Sources/App/entrypoint.swift Resolved
@fpseverino
fpseverino requested a review from 0xTim October 16, 2026 17:37
@fpseverino

Copy link
Copy Markdown
Member Author

OK, I think I’ve got a clearer picture now. The Swift Configuration guide on interoperability with ArgumentParser covers two scenarios:

  • Using only the CLI arguments defined in ParsableCommand and passing them to the ConfigReader, whilst rejecting any unrecognized ones (which is the default behaviour of ArgumentParser) (see here)
  • Letting the ParsableCommand handle its own arguments, and passing all any unrecognized ones (and only them, not the ones from the ParsableCommand) to the ConfigReader (see here)

The guide also states that it is possible to combine the two approaches if necessary, and this is precisely our situation, because we define the arguments that Vapor needs in the ParsableCommand, but we don’t retrieve them from the command itself; instead, we get them from the ConfigReader (first scenario).
Furthermore, we want any unrecognised arguments to be passed to ConfigReader so that other libraries can access them (second scenario).

My issue with combining the two approaches from the guide is that it's much more verbose and AFAICT technically the same as the code I wrote originally. It would be also be harder to maintain.

Here's the diff where I apply the Swift Configuration guidelines to my original code:

struct Serve: AsyncParsableCommand {
    @Option var hostname: String?
    @Option var port: Int?
    @Option var bind: String?
    @Option var socketPath: String?

    @Argument(parsing: .allUnrecognized, help: .private)
    var configArguments: [String] = []

    mutating func run() async throws {
+       var values: [AbsoluteConfigKey: ConfigValue] = [:]
+       if let hostname { values["hostname"] = .init(.string(hostname), isSecret: false) }
+       if let port { values["port"] = .init(.int(port), isSecret: false) }
+       if let bind { values["bind"] = .init(.string(bind), isSecret: false) }
+       if let socketPath { values["socketPath"] = .init(.string(socketPath), isSecret: false) }
        let config = ConfigReader(providers: [
+           InMemoryProvider(name: "arguments", values: values),
-           CommandLineArgumentsProvider(),
+           CommandLineArgumentsProvider(arguments: ["serve"] + configArguments),
            EnvironmentVariablesProvider(),
        ])
    }
}

The PR right now uses my version. I will ask the maintainers of Swift Configuration if my code is actually technically the same as the two approaches combined.

@fpseverino

Copy link
Copy Markdown
Member Author

I will ask the maintainers of Swift Configuration if my code is actually technically the same as the two approaches combined.

@0xTim I have received confirmation that this approach is valid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants