ArgumentHelp.Visibility levels API #390
Conversation
|
Some comments:
|
| /// the extended help display. | ||
| public var shouldDisplay: Bool = true |
There was a problem hiding this comment.
I dont think we can eliminate this property since it is part of our public API, but it can be deprecated and forward get/set to the new visibility property you've defined
Same for the initializer below
McNight
force-pushed
the
mcnight/argumenthelp_visibility_levels
branch
from
2c6eaec to
e2707e0
Compare
|
I made the modifications to keep and mark the |
| set { | ||
| visibility = newValue ? .default : .hidden | ||
| } | ||
| } | ||
|
|
||
| /// Creates a new help instance. | ||
| public init( |
There was a problem hiding this comment.
Could we add a depreciation to this initializer start moving away from shouldDisplay
There was a problem hiding this comment.
I marked this initializer as deprecated but didn't provide a deprecation message since I don't any a public alternative for now. However, I just learned (from you :)) about @_disfavoredOverload, do you think it could be useful here?
There was a problem hiding this comment.
I think the message should guide users to ArgumentHelp.init(_:discussion:valueName: visibility:). @_disfavoredOverload probably is needed as well since both inits can be called without providing any arguments.
There was a problem hiding this comment.
A message pointing at the new initializer would be great! If you remove the default value for shouldDisplay, you won't need to use @_disfavoredOverload.
McNight
force-pushed
the
mcnight/argumenthelp_visibility_levels
branch
from
e2707e0 to
f245342
Compare
There was a problem hiding this comment.
Looking good, @McNight! Don't worry about squashing commits, we do that when merging.
| set { | ||
| visibility = newValue ? .default : .hidden | ||
| } | ||
| } | ||
|
|
||
| /// Creates a new help instance. | ||
| public init( |
There was a problem hiding this comment.
A message pointing at the new initializer would be great! If you remove the default value for shouldDisplay, you won't need to use @_disfavoredOverload.
| @@ -11,6 +11,17 @@ | |||
|
|
|||
| /// Help information for a command-line argument. | |||
| public struct ArgumentHelp { | |||
| public enum Visibility { | |||
There was a problem hiding this comment.
Could you add an abstract for this type, as well?
McNight
force-pushed
the
mcnight/argumenthelp_visibility_levels
branch
from
f245342 to
6357b67
Compare
|
@swift-ci Please test |
This reverts commit 4cdcc17.
This pull request (as a draft for now) adds a new
ArgumentHelp.VisibilityAPI with multiple levels allowing to indicate whether help messages should be shown in the extended help display.It follows suggestions made by @natecook1000 in issue #384.
Checklist