impr: use enum definitions to explain options in usage #374
Conversation
|
I was also pointed to https://github.com/apple/swift-argument-parser/blame/10d80282e5fb5ae41338b54aeee5c9c5efc102ce/Sources/ArgumentParser/Parsable%20Types/ExpressibleByArgument.swift#L30-L39 but it hasn't shipped in an official release of ArgumentParser yet. Also because Renderer needs to be defined in XcbeautifyLib, I had to add Here's what it'd look like to make those changes from this branch: armcknight#2 |
There was a problem hiding this comment.
Hey @armcknight 👋 This is great! Thank you for making this contribution and also linking to the upcoming Swift Argument Parser changes (TIL). I left a few comments, but otherwise LGTM!
Sources/xcbeautify/Xcbeautify.swift
Outdated
| @@ -34,7 +35,7 @@ struct Xcbeautify: ParsableCommand { | |||
|
|
|||
| // swiftformat:disable redundantReturn | |||
|
|
|||
| @Option(help: "Specify a renderer to format raw xcodebuild output ( options: terminal | github-actions | teamcity | azure-devops-pipelines).") | |||
| @Option(help: "Specify a renderer to format raw xcodebuild output. (Options: \(Renderer.optionsDescription)). (Default: terminal).") | |||
There was a problem hiding this comment.
Hmm... terminal is not actually the default value (e.g. if we're running via GitHub Actions). I wonder if there's a succinct way for us to explain that renderer is computed based on environment values and uses terminal as a fallback. Any other ideas?
There was a problem hiding this comment.
Happy to report that there is also a property ExpressibleByArgument.defaultValueDescription already available that can cover this use case:
There was a problem hiding this comment.
For now though, I'll just leave it as it was in the past, with no default stated in the usage.
There was a problem hiding this comment.
Very nice – another TIL! Looking forward to the new Swift Argument Parser release, whenever the happens 🙄, but I agree leaving without default info is probably the best choice until then. Thanks!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #374 +/- ##
==========================================
+ Coverage 90.91% 91.20% +0.29%
==========================================
Files 17 18 +1
Lines 2454 2444 -10
==========================================
- Hits 2231 2229 -2
+ Misses 223 215 -8 ☔ View full report in Codecov by Sentry. |
|
@armcknight You can auto-fix the SwiftFormat issues with |
|
Thanks for reviewing, good suggestions @cpisciotta , ready! |
There was a problem hiding this comment.
Great work @armcknight 👏 Thanks so much for your contribution!
|
@armcknight This is available starting with 2.25.0. Homebrew should pick it up shortly. |
Hi, first contribution here, thanks for making this tool available 👋🏻
I hadn't noticed what the options were for
--reportby only reading the usage help (xcbeautify -h), and saw what it was when reading the source. I figured it'd be nice to add that to the usage.Then, I figured it'd be more maintainable if we used the enums themselves to describe to the help strings what the possible options are.
Thanks!