update System.CommandLine to the latest version#68549
Conversation
stop using System.CommandLine.NamingConventionBinder as it's reflection based and won't be supported in the long term stop using ancient System.CommandLine.Experimental
ghost
added
Area-Infrastructure
untriaged
| <add key="dotnet8" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8/nuget/v3/index.json" /> | ||
| <add key="dotnet8-transport" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8-transport/nuget/v3/index.json" /> | ||
| <add key="dotnet-public" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public/nuget/v3/index.json" /> | ||
| <add key="dotnet-libraries" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-libraries/nuget/v3/index.json" /> |
There was a problem hiding this comment.
Nine different dotnet feeds now ... I feel like we're making NuGet unnecessarily complicated.
|
|
||
| var parser = CreateCommandLineParser(); | ||
| return await parser.InvokeAsync(args); | ||
| return await parser.Parse(args).InvokeAsync(CancellationToken.None); |
There was a problem hiding this comment.
What cancellation token should be going in here? Was the old behavior listening to Ctrl+C to raise the token or not?
There was a problem hiding this comment.
The docs show InvokeAsync with no cancellationtoken. Might be one of our analyzers telling it to pass a cancellation token?
https://learn.microsoft.com/en-us/dotnet/standard/commandline/handle-termination
There was a problem hiding this comment.
Whatever token we pass here, S.CL is going to create a source linked to it:
and create it's own token out of it, then request the cancellation when its Ctrl+C is pressed
jasonmalinowski
left a comment
jasonmalinowski
left a comment
There was a problem hiding this comment.
Generally looks OK, some questions about cancellation tokens but that could be done as a follow-up.
| var logLevelOption = new CliOption<LogLevel>("--logLevel") | ||
| { | ||
| var value = result.Tokens.Single().Value; | ||
| return !Enum.TryParse<LogLevel>(value, out var logLevel) |
There was a problem hiding this comment.
@dibarbet Were enums just not supported on the version we were on or was this necessary for some special reason?
There was a problem hiding this comment.
I honestly don't remember. If this works with VSCode then I'm good with it.
There was a problem hiding this comment.
enums are now supported OOTB, I can not speak about the past as I've been involved in this project only for a year and a half
| cancellationToken); | ||
| }); | ||
|
|
||
| return generateCommand.Parse(args).InvokeAsync(CancellationToken.None); |
There was a problem hiding this comment.
Same question as earlier: what should go here for the token?
There was a problem hiding this comment.
Same question as earlier: what should go here for the token?
If this is Main, then None.
If this was any other async method that would have it's own token, then that token.
6 participants
stop using System.CommandLine.NamingConventionBinder as it's reflection based and won't be supported in the long term
stop using ancient System.CommandLine.Experimental
Contributes to dotnet/sdk#33157