Unify @Argument and @Option initialization paths #477
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rauhul
changed the title
rauhul
changed the title
rauhul
force-pushed
the
fix-446
branch
2 times, most recently
from
5a1e4b7
to
b45568b
Compare
rauhul
changed the title
|
@swift-ci please test |
natecook1000
reviewed
Sep 9, 2022
There was a problem hiding this comment.
This looks great — thanks in particular for the extra test coverage 👏🏻👏🏻👏🏻
I don't feel like we have a great way to know exactly whether APIs are being introduced / removed here. I ran the breaking changes diagnostic, and after stripping some spurious errors (it doesn't think that changing the name a generic parameter is okay), I'm still seeing these four breaks — could you take a look to see if these are real or not?
💔 API breakage: constructor Argument.init(wrappedValue:parsing:help:completion:) has parameter 0 type change from Value to [Element]
💔 API breakage: constructor Argument.init(wrappedValue:parsing:help:completion:transform:) has parameter 0 type change from Value to [Element]
💔 API breakage: constructor Option.init(wrappedValue:name:parsing:help:completion:) has parameter 0 type change from [Element] to Value
💔 API breakage: constructor Option.init(wrappedValue:name:parsing:help:completion:transform:) has parameter 0 type change from [Element] to Value
Tests/ArgumentParserUnitTests/HelpGenerationTests+AtArgument.swift
Outdated
Show resolved
Hide resolved
Comment on lines
+5
to
+6
| HelpGenerationTests+AtArgument.swift | ||
| HelpGenerationTests+AtOption.swift |
4 tasks
- Fixes #466. - Adds initializers to ArgumentDefinition generic over a Container type. The Container type must conform to a new internal protocol ArgumentDefinitionContainer which describes functionality like default set of help options for the argument defined by the property wrapper, etc. - Adds overloads for Optional @arguments and @options with default values which emit deprecation warning to guide users towards using the non-Optional versions.
|
@natecook1000 those are all spurious AFAICT, I reran locally to verify again. |
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
2 tasks
natecook1000
added a commit
that referenced
this pull request
Feb 29, 2024
Due to the restructuring in #477, there was ambiguity between the unconstrained `@Option` initializer that uses a transform (but no initial value) and the one that is constrained to the property being optional. This marks the unconstrained version as disfavored, which allows overload resolution to select the optional version when appropriate. Fixes #618.
4 tasks
natecook1000
added a commit
that referenced
this pull request
Mar 5, 2024
Due to the restructuring in #477, there was ambiguity between the unconstrained `@Option` initializer that uses a transform (but no initial value) and the one that is constrained to the property being optional. This marks the unconstrained version as disfavored, which allows overload resolution to select the optional version when appropriate. Also fixes this for `@Argument` and improves documentation consistency for `@Option`. Fixes #618.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
transform:parameter in@Argumentdeclaration generates wrong help message for optional arguments #466.The Container type must conform to a new internal protocol
ArgumentDefinitionContainer which describes functionality like default
set of help options for the argument defined by the property wrapper,
etc.
values which emit deprecation warning to guide users towards using the
non-Optional versions.