Hide counter argument and update --counters option description #1770
Conversation
|
CC @jonsequitur for S.CommandLine behavior Just to make sure I've got this straight: |
That's correct. |
josalem
left a comment
josalem
left a comment
There was a problem hiding this comment.
We'll need to call this out in the change log. It's not technically a breaking change, but we should discuss deprecation of arguments.
The arg change was already introduced in 5.0 release that went public. Docs were all changed to use the new argument, so hopefully it should nudge them towards using the correct argument. The old argument is also still supported, just not recommended : ) |
|
|
||
| private static Argument CounterList() => | ||
| new Argument<List<string>>(name: "counter_list", getDefaultValue: () => new List<string>() ) | ||
| new Argument<List<string>>(name: "counter_list", getDefaultValue: () => new List<string>()) |
There was a problem hiding this comment.
The getDefaultValue shouldn't be necessary here.
4 participants
Related issue: #1754
The underlying cause in the issue is System.CommandLine simply calling .ToString() on the default argument. This can be fixed by defining a CounterList class and overriding the ToString() (very hacky, yes...), but will be addressed in a different PR, if at all.
I think the best way to address it instead is to deprecate this argument because we now have the
--countersoption which is the preferred way to specify the counters.This also updates the description for the
--countersargument.